From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtprelay.hostedemail.com (smtprelay0073.hostedemail.com [216.40.44.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CF0D472 for ; Sat, 15 May 2021 13:03:28 +0000 (UTC) Received: from smtprelay.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by smtpgrave07.hostedemail.com (Postfix) with ESMTP id D850618029154 for ; Fri, 14 May 2021 15:56:19 +0000 (UTC) Received: from omf12.hostedemail.com (clb03-v110.bra.tucows.net [216.40.38.60]) by smtprelay05.hostedemail.com (Postfix) with ESMTP id EF2431802B4D5; Fri, 14 May 2021 15:56:12 +0000 (UTC) Received: from [HIDDEN] (Authenticated sender: joe@perches.com) by omf12.hostedemail.com (Postfix) with ESMTPA id 9398824023B; Fri, 14 May 2021 15:56:11 +0000 (UTC) Message-ID: Subject: Re: [PATCH] staging: greybus: fix gb_loopback_stats_attrs definition From: Joe Perches To: Greg KH , Shreyansh Chouhan Cc: pure.logic@nexus-software.ie, johan@kernel.org, elder@kernel.org, greybus-dev@lists.linaro.org, linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org Date: Fri, 14 May 2021 08:56:10 -0700 In-Reply-To: References: <20210514133039.304760-1-chouhan.shreyansh630@gmail.com> Content-Type: text/plain; charset="ISO-8859-1" User-Agent: Evolution 3.38.1-1 X-Mailing-List: linux-staging@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspamout01 X-Rspamd-Queue-Id: 9398824023B X-Spam-Status: No, score=-1.40 X-Stat-Signature: bsd4jy9waaadwjctjaqreet6ob39ob4w X-Session-Marker: 6A6F6540706572636865732E636F6D X-Session-ID: U2FsdGVkX19CiF0j1HLMKl+eajSpiUzTVTMqH8HjOIk= X-HE-Tag: 1621007771-347867 On Fri, 2021-05-14 at 17:30 +0200, Greg KH wrote: > On Fri, May 14, 2021 at 08:42:16PM +0530, Shreyansh Chouhan wrote: [] > > I didn't look at how/where was the macro called and missed a very > > obvious error. Now that I have looked at it, the only way I can think of > > fixing this is changing the macro to a (inline?) function. Will > > that be a desirable change? > > No, it can't be a function, the code is fine as-is, checkpatch is just a > perl script and does not always know what needs to be done. true. perhaps better though to rename these declaring macros to start with declare_ Something like this: (with miscellaneous realigning of the macros line ending continuations \) --- drivers/staging/greybus/loopback.c | 72 +++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index 2471448ba42a..dc399792f35f 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -119,18 +119,18 @@ module_param(kfifo_depth, uint, 0444); #define GB_LOOPBACK_US_WAIT_MAX 1000000 /* interface sysfs attributes */ -#define gb_loopback_ro_attr(field) \ -static ssize_t field##_show(struct device *dev, \ +#define declare_gb_loopback_ro_attr(field) \ +static ssize_t field##_show(struct device *dev, \ struct device_attribute *attr, \ char *buf) \ { \ struct gb_loopback *gb = dev_get_drvdata(dev); \ - return sprintf(buf, "%u\n", gb->field); \ + return sprintf(buf, "%u\n", gb->field); \ } \ static DEVICE_ATTR_RO(field) -#define gb_loopback_ro_stats_attr(name, field, type) \ -static ssize_t name##_##field##_show(struct device *dev, \ +#define declare_gb_loopback_ro_stats_attr(name, field, type) \ +static ssize_t name##_##field##_show(struct device *dev, \ struct device_attribute *attr, \ char *buf) \ { \ @@ -142,8 +142,8 @@ static ssize_t name##_##field##_show(struct device *dev, \ } \ static DEVICE_ATTR_RO(name##_##field) -#define gb_loopback_ro_avg_attr(name) \ -static ssize_t name##_avg_show(struct device *dev, \ +#define declare_gb_loopback_ro_avg_attr(name) \ +static ssize_t name##_avg_show(struct device *dev, \ struct device_attribute *attr, \ char *buf) \ { \ @@ -151,8 +151,8 @@ static ssize_t name##_avg_show(struct device *dev, \ struct gb_loopback *gb; \ u64 avg, rem; \ u32 count; \ - gb = dev_get_drvdata(dev); \ - stats = &gb->name; \ + gb = dev_get_drvdata(dev); \ + stats = &gb->name; \ count = stats->count ? stats->count : 1; \ avg = stats->sum + count / 2000000; /* round closest */ \ rem = do_div(avg, count); \ @@ -162,12 +162,12 @@ static ssize_t name##_avg_show(struct device *dev, \ } \ static DEVICE_ATTR_RO(name##_avg) -#define gb_loopback_stats_attrs(field) \ - gb_loopback_ro_stats_attr(field, min, u); \ - gb_loopback_ro_stats_attr(field, max, u); \ - gb_loopback_ro_avg_attr(field) +#define declare_gb_loopback_stats_attrs(field) \ + declare_gb_loopback_ro_stats_attr(field, min, u); \ + declare_gb_loopback_ro_stats_attr(field, max, u); \ + declare_gb_loopback_ro_avg_attr(field) -#define gb_loopback_attr(field, type) \ +#define declare_gb_loopback_attr(field, type) \ static ssize_t field##_show(struct device *dev, \ struct device_attribute *attr, \ char *buf) \ @@ -193,8 +193,8 @@ static ssize_t field##_store(struct device *dev, \ } \ static DEVICE_ATTR_RW(field) -#define gb_dev_loopback_ro_attr(field, conn) \ -static ssize_t field##_show(struct device *dev, \ +#define declare_gb_dev_loopback_ro_attr(field, conn) \ +static ssize_t field##_show(struct device *dev, \ struct device_attribute *attr, \ char *buf) \ { \ @@ -203,7 +203,7 @@ static ssize_t field##_show(struct device *dev, \ } \ static DEVICE_ATTR_RO(field) -#define gb_dev_loopback_rw_attr(field, type) \ +#define declare_gb_dev_loopback_rw_attr(field, type) \ static ssize_t field##_show(struct device *dev, \ struct device_attribute *attr, \ char *buf) \ @@ -223,7 +223,7 @@ static ssize_t field##_store(struct device *dev, \ if (ret != 1) \ len = -EINVAL; \ else \ - gb_loopback_check_attr(gb); \ + gb_loopback_check_attr(gb); \ mutex_unlock(&gb->mutex); \ return len; \ } \ @@ -268,26 +268,26 @@ static void gb_loopback_check_attr(struct gb_loopback *gb) } /* Time to send and receive one message */ -gb_loopback_stats_attrs(latency); +declare_gb_loopback_stats_attrs(latency); /* Number of requests sent per second on this cport */ -gb_loopback_stats_attrs(requests_per_second); +declare_gb_loopback_stats_attrs(requests_per_second); /* Quantity of data sent and received on this cport */ -gb_loopback_stats_attrs(throughput); +declare_gb_loopback_stats_attrs(throughput); /* Latency across the UniPro link from APBridge's perspective */ -gb_loopback_stats_attrs(apbridge_unipro_latency); +declare_gb_loopback_stats_attrs(apbridge_unipro_latency); /* Firmware induced overhead in the GPBridge */ -gb_loopback_stats_attrs(gbphy_firmware_latency); +declare_gb_loopback_stats_attrs(gbphy_firmware_latency); /* Number of errors encountered during loop */ -gb_loopback_ro_attr(error); +declare_gb_loopback_ro_attr(error); /* Number of requests successfully completed async */ -gb_loopback_ro_attr(requests_completed); +declare_gb_loopback_ro_attr(requests_completed); /* Number of requests timed out async */ -gb_loopback_ro_attr(requests_timedout); +declare_gb_loopback_ro_attr(requests_timedout); /* Timeout minimum in useconds */ -gb_loopback_ro_attr(timeout_min); +declare_gb_loopback_ro_attr(timeout_min); /* Timeout minimum in useconds */ -gb_loopback_ro_attr(timeout_max); +declare_gb_loopback_ro_attr(timeout_max); /* * Type of loopback message to send based on protocol type definitions @@ -297,21 +297,21 @@ gb_loopback_ro_attr(timeout_max); * payload returned in response) * 4 => Send a sink message (message with payload, no payload in response) */ -gb_dev_loopback_rw_attr(type, d); +declare_gb_dev_loopback_rw_attr(type, d); /* Size of transfer message payload: 0-4096 bytes */ -gb_dev_loopback_rw_attr(size, u); +declare_gb_dev_loopback_rw_attr(size, u); /* Time to wait between two messages: 0-1000 ms */ -gb_dev_loopback_rw_attr(us_wait, d); +declare_gb_dev_loopback_rw_attr(us_wait, d); /* Maximum iterations for a given operation: 1-(2^32-1), 0 implies infinite */ -gb_dev_loopback_rw_attr(iteration_max, u); +declare_gb_dev_loopback_rw_attr(iteration_max, u); /* The current index of the for (i = 0; i < iteration_max; i++) loop */ -gb_dev_loopback_ro_attr(iteration_count, false); +declare_gb_dev_loopback_ro_attr(iteration_count, false); /* A flag to indicate synchronous or asynchronous operations */ -gb_dev_loopback_rw_attr(async, u); +declare_gb_dev_loopback_rw_attr(async, u); /* Timeout of an individual asynchronous request */ -gb_dev_loopback_rw_attr(timeout, u); +declare_gb_dev_loopback_rw_attr(timeout, u); /* Maximum number of in-flight operations before back-off */ -gb_dev_loopback_rw_attr(outstanding_operations_max, u); +declare_gb_dev_loopback_rw_attr(outstanding_operations_max, u); static struct attribute *loopback_attrs[] = { &dev_attr_latency_min.attr, From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,UNPARSEABLE_RELAY autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 534B0C433B4 for ; Fri, 14 May 2021 15:56:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3797561444 for ; Fri, 14 May 2021 15:56:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234994AbhENP51 (ORCPT ); Fri, 14 May 2021 11:57:27 -0400 Received: from smtprelay0211.hostedemail.com ([216.40.44.211]:33766 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S231485AbhENP5Y (ORCPT ); Fri, 14 May 2021 11:57:24 -0400 Received: from omf12.hostedemail.com (clb03-v110.bra.tucows.net [216.40.38.60]) by smtprelay05.hostedemail.com (Postfix) with ESMTP id EF2431802B4D5; Fri, 14 May 2021 15:56:12 +0000 (UTC) Received: from [HIDDEN] (Authenticated sender: joe@perches.com) by omf12.hostedemail.com (Postfix) with ESMTPA id 9398824023B; Fri, 14 May 2021 15:56:11 +0000 (UTC) Message-ID: Subject: Re: [PATCH] staging: greybus: fix gb_loopback_stats_attrs definition From: Joe Perches To: Greg KH , Shreyansh Chouhan Cc: pure.logic@nexus-software.ie, johan@kernel.org, elder@kernel.org, greybus-dev@lists.linaro.org, linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org Date: Fri, 14 May 2021 08:56:10 -0700 In-Reply-To: References: <20210514133039.304760-1-chouhan.shreyansh630@gmail.com> Content-Type: text/plain; charset="ISO-8859-1" User-Agent: Evolution 3.38.1-1 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspamout01 X-Rspamd-Queue-Id: 9398824023B X-Stat-Signature: bsd4jy9waaadwjctjaqreet6ob39ob4w X-Session-Marker: 6A6F6540706572636865732E636F6D X-Session-ID: U2FsdGVkX19CiF0j1HLMKl+eajSpiUzTVTMqH8HjOIk= X-HE-Tag: 1621007771-347867 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2021-05-14 at 17:30 +0200, Greg KH wrote: > On Fri, May 14, 2021 at 08:42:16PM +0530, Shreyansh Chouhan wrote: [] > > I didn't look at how/where was the macro called and missed a very > > obvious error. Now that I have looked at it, the only way I can think of > > fixing this is changing the macro to a (inline?) function. Will > > that be a desirable change? > > No, it can't be a function, the code is fine as-is, checkpatch is just a > perl script and does not always know what needs to be done. true. perhaps better though to rename these declaring macros to start with declare_ Something like this: (with miscellaneous realigning of the macros line ending continuations \) --- drivers/staging/greybus/loopback.c | 72 +++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index 2471448ba42a..dc399792f35f 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -119,18 +119,18 @@ module_param(kfifo_depth, uint, 0444); #define GB_LOOPBACK_US_WAIT_MAX 1000000 /* interface sysfs attributes */ -#define gb_loopback_ro_attr(field) \ -static ssize_t field##_show(struct device *dev, \ +#define declare_gb_loopback_ro_attr(field) \ +static ssize_t field##_show(struct device *dev, \ struct device_attribute *attr, \ char *buf) \ { \ struct gb_loopback *gb = dev_get_drvdata(dev); \ - return sprintf(buf, "%u\n", gb->field); \ + return sprintf(buf, "%u\n", gb->field); \ } \ static DEVICE_ATTR_RO(field) -#define gb_loopback_ro_stats_attr(name, field, type) \ -static ssize_t name##_##field##_show(struct device *dev, \ +#define declare_gb_loopback_ro_stats_attr(name, field, type) \ +static ssize_t name##_##field##_show(struct device *dev, \ struct device_attribute *attr, \ char *buf) \ { \ @@ -142,8 +142,8 @@ static ssize_t name##_##field##_show(struct device *dev, \ } \ static DEVICE_ATTR_RO(name##_##field) -#define gb_loopback_ro_avg_attr(name) \ -static ssize_t name##_avg_show(struct device *dev, \ +#define declare_gb_loopback_ro_avg_attr(name) \ +static ssize_t name##_avg_show(struct device *dev, \ struct device_attribute *attr, \ char *buf) \ { \ @@ -151,8 +151,8 @@ static ssize_t name##_avg_show(struct device *dev, \ struct gb_loopback *gb; \ u64 avg, rem; \ u32 count; \ - gb = dev_get_drvdata(dev); \ - stats = &gb->name; \ + gb = dev_get_drvdata(dev); \ + stats = &gb->name; \ count = stats->count ? stats->count : 1; \ avg = stats->sum + count / 2000000; /* round closest */ \ rem = do_div(avg, count); \ @@ -162,12 +162,12 @@ static ssize_t name##_avg_show(struct device *dev, \ } \ static DEVICE_ATTR_RO(name##_avg) -#define gb_loopback_stats_attrs(field) \ - gb_loopback_ro_stats_attr(field, min, u); \ - gb_loopback_ro_stats_attr(field, max, u); \ - gb_loopback_ro_avg_attr(field) +#define declare_gb_loopback_stats_attrs(field) \ + declare_gb_loopback_ro_stats_attr(field, min, u); \ + declare_gb_loopback_ro_stats_attr(field, max, u); \ + declare_gb_loopback_ro_avg_attr(field) -#define gb_loopback_attr(field, type) \ +#define declare_gb_loopback_attr(field, type) \ static ssize_t field##_show(struct device *dev, \ struct device_attribute *attr, \ char *buf) \ @@ -193,8 +193,8 @@ static ssize_t field##_store(struct device *dev, \ } \ static DEVICE_ATTR_RW(field) -#define gb_dev_loopback_ro_attr(field, conn) \ -static ssize_t field##_show(struct device *dev, \ +#define declare_gb_dev_loopback_ro_attr(field, conn) \ +static ssize_t field##_show(struct device *dev, \ struct device_attribute *attr, \ char *buf) \ { \ @@ -203,7 +203,7 @@ static ssize_t field##_show(struct device *dev, \ } \ static DEVICE_ATTR_RO(field) -#define gb_dev_loopback_rw_attr(field, type) \ +#define declare_gb_dev_loopback_rw_attr(field, type) \ static ssize_t field##_show(struct device *dev, \ struct device_attribute *attr, \ char *buf) \ @@ -223,7 +223,7 @@ static ssize_t field##_store(struct device *dev, \ if (ret != 1) \ len = -EINVAL; \ else \ - gb_loopback_check_attr(gb); \ + gb_loopback_check_attr(gb); \ mutex_unlock(&gb->mutex); \ return len; \ } \ @@ -268,26 +268,26 @@ static void gb_loopback_check_attr(struct gb_loopback *gb) } /* Time to send and receive one message */ -gb_loopback_stats_attrs(latency); +declare_gb_loopback_stats_attrs(latency); /* Number of requests sent per second on this cport */ -gb_loopback_stats_attrs(requests_per_second); +declare_gb_loopback_stats_attrs(requests_per_second); /* Quantity of data sent and received on this cport */ -gb_loopback_stats_attrs(throughput); +declare_gb_loopback_stats_attrs(throughput); /* Latency across the UniPro link from APBridge's perspective */ -gb_loopback_stats_attrs(apbridge_unipro_latency); +declare_gb_loopback_stats_attrs(apbridge_unipro_latency); /* Firmware induced overhead in the GPBridge */ -gb_loopback_stats_attrs(gbphy_firmware_latency); +declare_gb_loopback_stats_attrs(gbphy_firmware_latency); /* Number of errors encountered during loop */ -gb_loopback_ro_attr(error); +declare_gb_loopback_ro_attr(error); /* Number of requests successfully completed async */ -gb_loopback_ro_attr(requests_completed); +declare_gb_loopback_ro_attr(requests_completed); /* Number of requests timed out async */ -gb_loopback_ro_attr(requests_timedout); +declare_gb_loopback_ro_attr(requests_timedout); /* Timeout minimum in useconds */ -gb_loopback_ro_attr(timeout_min); +declare_gb_loopback_ro_attr(timeout_min); /* Timeout minimum in useconds */ -gb_loopback_ro_attr(timeout_max); +declare_gb_loopback_ro_attr(timeout_max); /* * Type of loopback message to send based on protocol type definitions @@ -297,21 +297,21 @@ gb_loopback_ro_attr(timeout_max); * payload returned in response) * 4 => Send a sink message (message with payload, no payload in response) */ -gb_dev_loopback_rw_attr(type, d); +declare_gb_dev_loopback_rw_attr(type, d); /* Size of transfer message payload: 0-4096 bytes */ -gb_dev_loopback_rw_attr(size, u); +declare_gb_dev_loopback_rw_attr(size, u); /* Time to wait between two messages: 0-1000 ms */ -gb_dev_loopback_rw_attr(us_wait, d); +declare_gb_dev_loopback_rw_attr(us_wait, d); /* Maximum iterations for a given operation: 1-(2^32-1), 0 implies infinite */ -gb_dev_loopback_rw_attr(iteration_max, u); +declare_gb_dev_loopback_rw_attr(iteration_max, u); /* The current index of the for (i = 0; i < iteration_max; i++) loop */ -gb_dev_loopback_ro_attr(iteration_count, false); +declare_gb_dev_loopback_ro_attr(iteration_count, false); /* A flag to indicate synchronous or asynchronous operations */ -gb_dev_loopback_rw_attr(async, u); +declare_gb_dev_loopback_rw_attr(async, u); /* Timeout of an individual asynchronous request */ -gb_dev_loopback_rw_attr(timeout, u); +declare_gb_dev_loopback_rw_attr(timeout, u); /* Maximum number of in-flight operations before back-off */ -gb_dev_loopback_rw_attr(outstanding_operations_max, u); +declare_gb_dev_loopback_rw_attr(outstanding_operations_max, u); static struct attribute *loopback_attrs[] = { &dev_attr_latency_min.attr,