Message ID | 20231221175527.2814506-1-andriy.shevchenko@linux.intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-8954-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:2483:b0:fb:cd0c:d3e with SMTP id q3csp618936dyi; Thu, 21 Dec 2023 10:51:13 -0800 (PST) X-Google-Smtp-Source: AGHT+IHpsf7T57g52rK/qzXvdiYWV41QxpvQaAJNPTep87VV8tNvt2LNP+V5a/Bj8SpBa5uH+O+A X-Received: by 2002:ac8:5d87:0:b0:427:a570:d55c with SMTP id d7-20020ac85d87000000b00427a570d55cmr289831qtx.0.1703184673605; Thu, 21 Dec 2023 10:51:13 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1703184673; cv=none; d=google.com; s=arc-20160816; b=d/rxaGPMBQ9EEggsN+xvwvazcqC1qYquL3BDM8E0u8xXOjRt+ozfEmtq6ZXrNyTIXD p5QpA7v8bZYWY8GiAtZAIuPrMAaiF/pkitULOpe4EPfLmzNeYuEyjBhWe3G9Y/qnddfh dq8QmCtRGj5nzcB3GfpKnrHy5sCY4hzsQ+P3Ml9oAEEqKV5k2gdgAz+R8it3Tm3c1HBo T6D0GOlx/HUA6BePVeVbnQrgTyz+lb8mdXlxteNIb+stOwXGTL9SgxbYcpvc0blAcIKu KY14hk0yD81LB4c5mW+5P0FKouD+ohtFEKA3fN3fW3kHuPdDDCVrcs6NSLKP5vJcCnmv KdfQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from:dkim-signature; bh=CohXqtdBZoZrm7L9quHD4aIGnijyC9hhE0Ni/gVOSMQ=; fh=6D+nIbsn1bKeSxYDS9idmz1tZNgsAz7RWbEC4njspmo=; b=anKkfjHItG0coyxLhqPDXJ355Tb+WdwNO00fzDGYVYa4SjVru6O20HA4B/w3pZXk7n CvHArT47lCp0amF8Kt/1yew6gualduxg2Bcoq+aIpD0fv3IH7jjB+YuT/Rr9suS4py1s WT+9vI7sj7hWK9ZOWesoph4u2qtT7U9IkpBm0/E6mDlvNu7E9arvnIsc8NGbBumVyK8S XOChPcrKqctPLqaxt0M3NHZz36LRiFdt6sxRbHOzoFsiRdi+iSkd3B6whD4ECFnKyYVd ssJVJjUatca2anVK8JRe76Mw7X+bUgAu4G2dOc+dWQiyFjsb7XFuHgAVhmp8EqDzVobM o1Jw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=NRSvr9xw; spf=pass (google.com: domain of linux-kernel+bounces-8954-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) smtp.mailfrom="linux-kernel+bounces-8954-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from ny.mirrors.kernel.org (ny.mirrors.kernel.org. [147.75.199.223]) by mx.google.com with ESMTPS id 16-20020ac85750000000b00427773174e2si2644498qtx.422.2023.12.21.10.51.13 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 21 Dec 2023 10:51:13 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-8954-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=NRSvr9xw; spf=pass (google.com: domain of linux-kernel+bounces-8954-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) smtp.mailfrom="linux-kernel+bounces-8954-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id 52AAD1C24003 for <ouuuleilei@gmail.com>; Thu, 21 Dec 2023 18:51:13 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id A924465197; Thu, 21 Dec 2023 18:50:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="NRSvr9xw" X-Original-To: linux-kernel@vger.kernel.org Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.100]) (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 77C636518B; Thu, 21 Dec 2023 18:50:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=linux.intel.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1703184631; x=1734720631; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=6lqfECsnx9YNGz7mNu1P1q5RcYfS8nYI6pc/nxTM7UQ=; b=NRSvr9xwu8uUsCJvIuUNMSq4vmzVfwdjoepfVThM+cj1amxFYXz1HfN4 aeGC0pFU0JNpOqZJu3snBaAIwYrRsY4Nhccg90CsH+07tC4mQV/NBdfiJ QyEOTYO0FyfXRDDos3biRWv+ktHvIiu220Xdla3tN9f13bb8OArZgoLMh eGW2nRaGFFrB5wXKXLd2kWYBu12kB1fEqGICz5xm95FsvMskLDNPQFjX/ XjX8U8qPFQJF5wu4q2+RZ7r+j80cRgSH9CsP4fmmZmlMzUU5PBLELdIg8 MILUq2Yp3fLFw2dnPLCar6VdLmL4yimdGQZboNThB1DKE1ntADzlqlC7s A==; X-IronPort-AV: E=McAfee;i="6600,9927,10931"; a="462463230" X-IronPort-AV: E=Sophos;i="6.04,294,1695711600"; d="scan'208";a="462463230" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Dec 2023 10:50:30 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10931"; a="920407804" X-IronPort-AV: E=Sophos;i="6.04,294,1695711600"; d="scan'208";a="920407804" Received: from black.fi.intel.com ([10.237.72.28]) by fmsmga001.fm.intel.com with ESMTP; 21 Dec 2023 10:50:28 -0800 Received: by black.fi.intel.com (Postfix, from userid 1003) id 93260B8; Thu, 21 Dec 2023 19:55:28 +0200 (EET) From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> To: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>, linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Linus Walleij <linus.walleij@linaro.org>, Bartosz Golaszewski <brgl@bgdev.pl>, Andy Shevchenko <andy@kernel.org>, Andy Shevchenko <andriy.shevchenko@linux.intel.com> Subject: [PATCH v1 1/1] gpiolib: cdev: Split line_get_debounce_period() and use Date: Thu, 21 Dec 2023 19:55:27 +0200 Message-ID: <20231221175527.2814506-1-andriy.shevchenko@linux.intel.com> X-Mailer: git-send-email 2.43.0.rc1.1.gbec44491f096 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1785918572358200940 X-GMAIL-MSGID: 1785918572358200940 |
Series |
[v1,1/1] gpiolib: cdev: Split line_get_debounce_period() and use
|
|
Commit Message
Andy Shevchenko
Dec. 21, 2023, 5:55 p.m. UTC
Instead of repeating the same code and reduce possible miss
of READ_ONCE(), split line_get_debounce_period() heler out
and use in the existing cases.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/gpio/gpiolib-cdev.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
Comments
On Thu, Dec 21, 2023 at 07:55:27PM +0200, Andy Shevchenko wrote: > Instead of repeating the same code and reduce possible miss > of READ_ONCE(), split line_get_debounce_period() heler out > and use in the existing cases. > helper Not a fan of this change. So using READ_ONCE() is repeating code?? Doesn't providing a wrapper around READ_ONCE() just rename that repitition? What of all the other uses of READ_ONCE() in cdev (and there are a lot) - why pick on debounce_period? The line_set_debounce_period() is necessary as the set is now a multi-step process as it can impact whether the line is contained in the supinfo_tree. The get is just a get. And you could've included me in the Cc so I didn't just find it by accident. Cheers, Kent. > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/gpio/gpiolib-cdev.c | 23 ++++++++++++++--------- > 1 file changed, 14 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c > index 744734405912..c573820d5722 100644 > --- a/drivers/gpio/gpiolib-cdev.c > +++ b/drivers/gpio/gpiolib-cdev.c > @@ -651,6 +651,16 @@ static struct line *supinfo_find(struct gpio_desc *desc) > return NULL; > } > > +static unsigned int line_get_debounce_period(struct line *line) > +{ > + return READ_ONCE(line->debounce_period_us); > +} > + > +static inline bool line_has_supinfo(struct line *line) > +{ > + return line_get_debounce_period(line); > +} > + > static void supinfo_to_lineinfo(struct gpio_desc *desc, > struct gpio_v2_line_info *info) > { > @@ -665,15 +675,10 @@ static void supinfo_to_lineinfo(struct gpio_desc *desc, > > attr = &info->attrs[info->num_attrs]; > attr->id = GPIO_V2_LINE_ATTR_ID_DEBOUNCE; > - attr->debounce_period_us = READ_ONCE(line->debounce_period_us); > + attr->debounce_period_us = line_get_debounce_period(line); > info->num_attrs++; > } > > -static inline bool line_has_supinfo(struct line *line) > -{ > - return READ_ONCE(line->debounce_period_us); > -} > - > /* > * Checks line_has_supinfo() before and after the change to avoid unnecessary > * supinfo_tree access. > @@ -846,7 +851,7 @@ static enum hte_return process_hw_ts(struct hte_ts_data *ts, void *p) > line->total_discard_seq++; > line->last_seqno = ts->seq; > mod_delayed_work(system_wq, &line->work, > - usecs_to_jiffies(READ_ONCE(line->debounce_period_us))); > + usecs_to_jiffies(line_get_debounce_period(line))); > } else { > if (unlikely(ts->seq < line->line_seqno)) > return HTE_CB_HANDLED; > @@ -987,7 +992,7 @@ static irqreturn_t debounce_irq_handler(int irq, void *p) > struct line *line = p; > > mod_delayed_work(system_wq, &line->work, > - usecs_to_jiffies(READ_ONCE(line->debounce_period_us))); > + usecs_to_jiffies(line_get_debounce_period(line))); > > return IRQ_HANDLED; > } > @@ -1215,7 +1220,7 @@ static int edge_detector_update(struct line *line, > gpio_v2_line_config_debounce_period(lc, line_idx); > > if ((active_edflags == edflags) && > - (READ_ONCE(line->debounce_period_us) == debounce_period_us)) > + (line_get_debounce_period(line) == debounce_period_us)) > return 0; > > /* sw debounced and still will be...*/ > -- > 2.43.0.rc1.1.gbec44491f096 >
On Fri, Dec 22, 2023 at 2:12 AM Kent Gibson <warthog618@gmail.com> wrote: > > On Thu, Dec 21, 2023 at 07:55:27PM +0200, Andy Shevchenko wrote: > > Instead of repeating the same code and reduce possible miss > > of READ_ONCE(), split line_get_debounce_period() heler out > > and use in the existing cases. > > > > helper > > > Not a fan of this change. > Yeah, sorry but NAK. READ_ONCE() is well known and tells you what the code does. Arbitrary line_get_debounce_period() makes me have to look it up. Bart > So using READ_ONCE() is repeating code?? > Doesn't providing a wrapper around READ_ONCE() just rename that repitition? > What of all the other uses of READ_ONCE() in cdev (and there are a lot) - > why pick on debounce_period? > > The line_set_debounce_period() is necessary as the set is now a > multi-step process as it can impact whether the line is contained > in the supinfo_tree. The get is just a get. > > And you could've included me in the Cc so I didn't just find it by > accident. > > Cheers, > Kent. > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > --- > > drivers/gpio/gpiolib-cdev.c | 23 ++++++++++++++--------- > > 1 file changed, 14 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c > > index 744734405912..c573820d5722 100644 > > --- a/drivers/gpio/gpiolib-cdev.c > > +++ b/drivers/gpio/gpiolib-cdev.c > > @@ -651,6 +651,16 @@ static struct line *supinfo_find(struct gpio_desc *desc) > > return NULL; > > } > > > > +static unsigned int line_get_debounce_period(struct line *line) > > +{ > > + return READ_ONCE(line->debounce_period_us); > > +} > > + > > +static inline bool line_has_supinfo(struct line *line) > > +{ > > + return line_get_debounce_period(line); > > +} > > + > > static void supinfo_to_lineinfo(struct gpio_desc *desc, > > struct gpio_v2_line_info *info) > > { > > @@ -665,15 +675,10 @@ static void supinfo_to_lineinfo(struct gpio_desc *desc, > > > > attr = &info->attrs[info->num_attrs]; > > attr->id = GPIO_V2_LINE_ATTR_ID_DEBOUNCE; > > - attr->debounce_period_us = READ_ONCE(line->debounce_period_us); > > + attr->debounce_period_us = line_get_debounce_period(line); > > info->num_attrs++; > > } > > > > -static inline bool line_has_supinfo(struct line *line) > > -{ > > - return READ_ONCE(line->debounce_period_us); > > -} > > - > > /* > > * Checks line_has_supinfo() before and after the change to avoid unnecessary > > * supinfo_tree access. > > @@ -846,7 +851,7 @@ static enum hte_return process_hw_ts(struct hte_ts_data *ts, void *p) > > line->total_discard_seq++; > > line->last_seqno = ts->seq; > > mod_delayed_work(system_wq, &line->work, > > - usecs_to_jiffies(READ_ONCE(line->debounce_period_us))); > > + usecs_to_jiffies(line_get_debounce_period(line))); > > } else { > > if (unlikely(ts->seq < line->line_seqno)) > > return HTE_CB_HANDLED; > > @@ -987,7 +992,7 @@ static irqreturn_t debounce_irq_handler(int irq, void *p) > > struct line *line = p; > > > > mod_delayed_work(system_wq, &line->work, > > - usecs_to_jiffies(READ_ONCE(line->debounce_period_us))); > > + usecs_to_jiffies(line_get_debounce_period(line))); > > > > return IRQ_HANDLED; > > } > > @@ -1215,7 +1220,7 @@ static int edge_detector_update(struct line *line, > > gpio_v2_line_config_debounce_period(lc, line_idx); > > > > if ((active_edflags == edflags) && > > - (READ_ONCE(line->debounce_period_us) == debounce_period_us)) > > + (line_get_debounce_period(line) == debounce_period_us)) > > return 0; > > > > /* sw debounced and still will be...*/ > > -- > > 2.43.0.rc1.1.gbec44491f096 > >
On Fri, Dec 22, 2023 at 09:12:37AM +0800, Kent Gibson wrote: > On Thu, Dec 21, 2023 at 07:55:27PM +0200, Andy Shevchenko wrote: > > Instead of repeating the same code and reduce possible miss > > of READ_ONCE(), split line_get_debounce_period() heler out > > and use in the existing cases. > > > > helper > > Not a fan of this change. > > So using READ_ONCE() is repeating code?? Yes. Because one may forget about it. > Doesn't providing a wrapper around READ_ONCE() just rename that repitition? > What of all the other uses of READ_ONCE() in cdev (and there are a lot) - > why pick on debounce_period? Because you have a setter, but getter. Inconsistency. > The line_set_debounce_period() is necessary as the set is now a > multi-step process as it can impact whether the line is contained > in the supinfo_tree. The get is just a get. > > And you could've included me in the Cc so I didn't just find it by > accident. Maybe it's time to add you to the MAINTAINERS for this file as a designated reviewer?
On Fri, Dec 22, 2023 at 09:58:48AM +0100, Bartosz Golaszewski wrote: > On Fri, Dec 22, 2023 at 2:12 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > On Thu, Dec 21, 2023 at 07:55:27PM +0200, Andy Shevchenko wrote: > > > Instead of repeating the same code and reduce possible miss > > > of READ_ONCE(), split line_get_debounce_period() heler out > > > and use in the existing cases. > > > > > > > helper > > > > > > Not a fan of this change. > > > > Yeah, sorry but NAK. READ_ONCE() is well known and tells you what the > code does. Arbitrary line_get_debounce_period() makes me have to look > it up. We have setter, but not getter. It looks confusing, more over, the setter makes much more than just set. Hence another way to solve this is make clear (by changing name) that the setter is not _just_ a setter.
On Fri, Dec 22, 2023 at 02:39:38PM +0200, Andy Shevchenko wrote: > On Fri, Dec 22, 2023 at 09:12:37AM +0800, Kent Gibson wrote: > > On Thu, Dec 21, 2023 at 07:55:27PM +0200, Andy Shevchenko wrote: > > > Instead of repeating the same code and reduce possible miss > > > of READ_ONCE(), split line_get_debounce_period() heler out > > > and use in the existing cases. > > > > > > > helper > > > > Not a fan of this change. > > > > So using READ_ONCE() is repeating code?? > > Yes. Because one may forget about it. Just as one may forget to use your wrapper. This argument is a NULL - so I'll just forget about it. > > > Doesn't providing a wrapper around READ_ONCE() just rename that repitition? > > What of all the other uses of READ_ONCE() in cdev (and there are a lot) - > > why pick on debounce_period? > > Because you have a setter, but getter. Inconsistency. > But then "for consistency" ALL the struct line fields require accessors and mutators. That path is insanity. The setter is there as setting the value now has side effects - none of which are visible to the caller, hence the usage of the standard setter name. You are siggesting every function name describe everything the function does? And, in case you've forgotten, YOU REVIEWED THIS. > > The line_set_debounce_period() is necessary as the set is now a > > multi-step process as it can impact whether the line is contained > > in the supinfo_tree. The get is just a get. > > > > And you could've included me in the Cc so I didn't just find it by > > accident. > > Maybe it's time to add you to the MAINTAINERS for this file as a designated > reviewer? > You are patching my recent change that you yourself reviewed only days ago. I would think that you would Cc me whether I were a maintainer or not as I'm very likely to have relevant feedback. Cheers, Kent.
On Fri, Dec 22, 2023 at 02:40:59PM +0200, Andy Shevchenko wrote: > On Fri, Dec 22, 2023 at 09:58:48AM +0100, Bartosz Golaszewski wrote: > > On Fri, Dec 22, 2023 at 2:12 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > > > On Thu, Dec 21, 2023 at 07:55:27PM +0200, Andy Shevchenko wrote: > > > > Instead of repeating the same code and reduce possible miss > > > > of READ_ONCE(), split line_get_debounce_period() heler out > > > > and use in the existing cases. > > > > > > > > > > helper > > > > > > > > > Not a fan of this change. > > > > > > > Yeah, sorry but NAK. READ_ONCE() is well known and tells you what the > > code does. Arbitrary line_get_debounce_period() makes me have to look > > it up. > > We have setter, but not getter. It looks confusing, more over, the setter makes > much more than just set. Hence another way to solve this is make clear (by > changing name) that the setter is not _just_ a setter. > As I mentioned elsewhere, the side effects of the setter are irrelevant to the caller, so from their point of view it is _just_ a setter. Calling it something else would actually be more confusing. Cheers, Kent.
On Fri, Dec 22, 2023 at 1:56 PM Kent Gibson <warthog618@gmail.com> wrote: > > On Fri, Dec 22, 2023 at 02:39:38PM +0200, Andy Shevchenko wrote: > > On Fri, Dec 22, 2023 at 09:12:37AM +0800, Kent Gibson wrote: > > > On Thu, Dec 21, 2023 at 07:55:27PM +0200, Andy Shevchenko wrote: > > > > Instead of repeating the same code and reduce possible miss > > > > of READ_ONCE(), split line_get_debounce_period() heler out > > > > and use in the existing cases. > > > > > > > > > > helper > > > > > > Not a fan of this change. > > > > > > So using READ_ONCE() is repeating code?? > > > > Yes. Because one may forget about it. > > Just as one may forget to use your wrapper. > This argument is a NULL - so I'll just forget about it. > > > > > > Doesn't providing a wrapper around READ_ONCE() just rename that repitition? > > > What of all the other uses of READ_ONCE() in cdev (and there are a lot) - > > > why pick on debounce_period? > > > > Because you have a setter, but getter. Inconsistency. > > > > But then "for consistency" ALL the struct line fields require accessors > and mutators. That path is insanity. > > The setter is there as setting the value now has side effects - none of > which are visible to the caller, hence the usage of the standard > setter name. > You are siggesting every function name describe everything the function > does? > > And, in case you've forgotten, YOU REVIEWED THIS. > > > > The line_set_debounce_period() is necessary as the set is now a > > > multi-step process as it can impact whether the line is contained > > > in the supinfo_tree. The get is just a get. > > > > > > And you could've included me in the Cc so I didn't just find it by > > > accident. > > > > Maybe it's time to add you to the MAINTAINERS for this file as a designated > > reviewer? > > > > You are patching my recent change that you yourself reviewed only days > ago. I would think that you would Cc me whether I were a maintainer or > not as I'm very likely to have relevant feedback. On that note: do you see yourself as a full GPIO reviewer or do you prefer I split out the uAPI part into a separate section in MAINTAINERS and nominate you as its maintainer? Bart > > Cheers, > Kent.
On Fri, Dec 22, 2023 at 02:37:43PM +0100, Bartosz Golaszewski wrote: > On Fri, Dec 22, 2023 at 1:56 PM Kent Gibson <warthog618@gmail.com> wrote: > > > > > > And you could've included me in the Cc so I didn't just find it by > > > > accident. > > > > > > Maybe it's time to add you to the MAINTAINERS for this file as a designated > > > reviewer? > > > > > > > You are patching my recent change that you yourself reviewed only days > > ago. I would think that you would Cc me whether I were a maintainer or > > not as I'm very likely to have relevant feedback. > > On that note: do you see yourself as a full GPIO reviewer or do you > prefer I split out the uAPI part into a separate section in > MAINTAINERS and nominate you as its maintainer? > Not sure I'm comfortable with either. Definitely not full GPIO. I don't feel sufficiently familiar with GPIO and the related subsystems to qualify. Splitting out cdev and the uAPI makes more sense to me, but in my mind at least even that requires a level of commitment higher than the rather spotty attention I've been providing recently. I'm more inclined to leave it as is. Cheers, Kent.
On Fri, 22 Dec 2023 at 15:08, Kent Gibson <warthog618@gmail.com> wrote: > > On Fri, Dec 22, 2023 at 02:37:43PM +0100, Bartosz Golaszewski wrote: > > On Fri, Dec 22, 2023 at 1:56 PM Kent Gibson <warthog618@gmail.com> wrote: > > > > > > > > > And you could've included me in the Cc so I didn't just find it by > > > > > accident. > > > > > > > > Maybe it's time to add you to the MAINTAINERS for this file as a designated > > > > reviewer? > > > > > > > > > > You are patching my recent change that you yourself reviewed only days > > > ago. I would think that you would Cc me whether I were a maintainer or > > > not as I'm very likely to have relevant feedback. > > > > On that note: do you see yourself as a full GPIO reviewer or do you > > prefer I split out the uAPI part into a separate section in > > MAINTAINERS and nominate you as its maintainer? > > > > Not sure I'm comfortable with either. > > Definitely not full GPIO. I don't feel sufficiently familiar with GPIO > and the related subsystems to qualify. > > Splitting out cdev and the uAPI makes more sense to me, but in my mind at > least even that requires a level of commitment higher than the rather > spotty attention I've been providing recently. > I'm more inclined to leave it as is. > I can still split the uAPI files into their own section, make Linus and myself maintainers and make you a reviewer, how about that? Bart > Cheers, > Kent.
On Fri, Dec 22, 2023 at 03:09:54PM +0100, Bartosz Golaszewski wrote: > On Fri, 22 Dec 2023 at 15:08, Kent Gibson <warthog618@gmail.com> wrote: > > > > On Fri, Dec 22, 2023 at 02:37:43PM +0100, Bartosz Golaszewski wrote: > > > On Fri, Dec 22, 2023 at 1:56 PM Kent Gibson <warthog618@gmail.com> wrote: > > > > > > > > > > > > And you could've included me in the Cc so I didn't just find it by > > > > > > accident. > > > > > > > > > > Maybe it's time to add you to the MAINTAINERS for this file as a designated > > > > > reviewer? > > > > > > > > > > > > > You are patching my recent change that you yourself reviewed only days > > > > ago. I would think that you would Cc me whether I were a maintainer or > > > > not as I'm very likely to have relevant feedback. > > > > > > On that note: do you see yourself as a full GPIO reviewer or do you > > > prefer I split out the uAPI part into a separate section in > > > MAINTAINERS and nominate you as its maintainer? > > > > > > > Not sure I'm comfortable with either. > > > > Definitely not full GPIO. I don't feel sufficiently familiar with GPIO > > and the related subsystems to qualify. > > > > Splitting out cdev and the uAPI makes more sense to me, but in my mind at > > least even that requires a level of commitment higher than the rather > > spotty attention I've been providing recently. > > I'm more inclined to leave it as is. > > > > I can still split the uAPI files into their own section, make Linus > and myself maintainers and make you a reviewer, how about that? > That is closer to the reality, so that would work for me. Cheers, Kent.
On Fri, Dec 22, 2023 at 3:15 PM Kent Gibson <warthog618@gmail.com> wrote: > On Fri, Dec 22, 2023 at 03:09:54PM +0100, Bartosz Golaszewski wrote: > > I can still split the uAPI files into their own section, make Linus > > and myself maintainers and make you a reviewer, how about that? > > That is closer to the reality, so that would work for me. Hmm I think of Kent as one of the main architects for UAPI v2 so I would like you as maintainer, and me to be dropped, I already responded to that patch though. Yours, Linus Walleij
On Fri, Dec 22, 2023 at 06:49:03PM +0100, Linus Walleij wrote: > On Fri, Dec 22, 2023 at 3:15 PM Kent Gibson <warthog618@gmail.com> wrote: > > On Fri, Dec 22, 2023 at 03:09:54PM +0100, Bartosz Golaszewski wrote: > > > > I can still split the uAPI files into their own section, make Linus > > > and myself maintainers and make you a reviewer, how about that? > > > > That is closer to the reality, so that would work for me. > > Hmm I think of Kent as one of the main architects for UAPI v2 > so I would like you as maintainer, and me to be dropped, I already > responded to that patch though. > There is no escaping that my fingerprints are all over that so it does make sense to list me over you. Given that patch and git-tree management will be deferred to the GPIO subsystem/Bart, there isn't much distinction between a reviewer and a maintainer, so I'm ok with being listed as a maintainer - I'll just have to pay a bit more attention to the list mails than I have been. Cheers, Kent.
Hi Kent, On Sat, Dec 23, 2023 at 3:08 AM Kent Gibson <warthog618@gmail.com> wrote: > There is no escaping that my fingerprints are all over that so it does > make sense to list me over you. Given that patch and git-tree management > will be deferred to the GPIO subsystem/Bart, there isn't much distinction > between a reviewer and a maintainer, so I'm ok with being listed as a > maintainer - I'll just have to pay a bit more attention to the list mails > than I have been. You're doing fine with responsiveness and feedback is always timely and verbose, so just keep doing what you do :) Yours, Linus Walleij
On Thu, Dec 28, 2023 at 01:26:13AM +0100, Linus Walleij wrote: > Hi Kent, > > On Sat, Dec 23, 2023 at 3:08 AM Kent Gibson <warthog618@gmail.com> wrote: > > > There is no escaping that my fingerprints are all over that so it does > > make sense to list me over you. Given that patch and git-tree management > > will be deferred to the GPIO subsystem/Bart, there isn't much distinction > > between a reviewer and a maintainer, so I'm ok with being listed as a > > maintainer - I'll just have to pay a bit more attention to the list mails > > than I have been. > > You're doing fine with responsiveness and feedback is always timely > and verbose, so just keep doing what you do :) > I endeavour to reply to direct mail promptly, but I was thinking more of mail directed generally to the list and there have been times I haven't checked the list for months. Cheers, Kent.
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c index 744734405912..c573820d5722 100644 --- a/drivers/gpio/gpiolib-cdev.c +++ b/drivers/gpio/gpiolib-cdev.c @@ -651,6 +651,16 @@ static struct line *supinfo_find(struct gpio_desc *desc) return NULL; } +static unsigned int line_get_debounce_period(struct line *line) +{ + return READ_ONCE(line->debounce_period_us); +} + +static inline bool line_has_supinfo(struct line *line) +{ + return line_get_debounce_period(line); +} + static void supinfo_to_lineinfo(struct gpio_desc *desc, struct gpio_v2_line_info *info) { @@ -665,15 +675,10 @@ static void supinfo_to_lineinfo(struct gpio_desc *desc, attr = &info->attrs[info->num_attrs]; attr->id = GPIO_V2_LINE_ATTR_ID_DEBOUNCE; - attr->debounce_period_us = READ_ONCE(line->debounce_period_us); + attr->debounce_period_us = line_get_debounce_period(line); info->num_attrs++; } -static inline bool line_has_supinfo(struct line *line) -{ - return READ_ONCE(line->debounce_period_us); -} - /* * Checks line_has_supinfo() before and after the change to avoid unnecessary * supinfo_tree access. @@ -846,7 +851,7 @@ static enum hte_return process_hw_ts(struct hte_ts_data *ts, void *p) line->total_discard_seq++; line->last_seqno = ts->seq; mod_delayed_work(system_wq, &line->work, - usecs_to_jiffies(READ_ONCE(line->debounce_period_us))); + usecs_to_jiffies(line_get_debounce_period(line))); } else { if (unlikely(ts->seq < line->line_seqno)) return HTE_CB_HANDLED; @@ -987,7 +992,7 @@ static irqreturn_t debounce_irq_handler(int irq, void *p) struct line *line = p; mod_delayed_work(system_wq, &line->work, - usecs_to_jiffies(READ_ONCE(line->debounce_period_us))); + usecs_to_jiffies(line_get_debounce_period(line))); return IRQ_HANDLED; } @@ -1215,7 +1220,7 @@ static int edge_detector_update(struct line *line, gpio_v2_line_config_debounce_period(lc, line_idx); if ((active_edflags == edflags) && - (READ_ONCE(line->debounce_period_us) == debounce_period_us)) + (line_get_debounce_period(line) == debounce_period_us)) return 0; /* sw debounced and still will be...*/