Message ID | 20230307182557.42215-4-andriy.shevchenko@linux.intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:5915:0:0:0:0:0 with SMTP id v21csp2621788wrd; Tue, 7 Mar 2023 11:35:14 -0800 (PST) X-Google-Smtp-Source: AK7set/ZYDuKCM1KrY4y3vN6fdjQO9Rx/nqOiCYOKtLb3+InOXBLBxXC9U1b4dFetjHWNCbslSI9 X-Received: by 2002:a05:6a20:b91e:b0:cc:1184:6d0 with SMTP id fe30-20020a056a20b91e00b000cc118406d0mr12019979pzb.23.1678217713926; Tue, 07 Mar 2023 11:35:13 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1678217713; cv=none; d=google.com; s=arc-20160816; b=XhzTDrJTiaN9I7QH3P1aZdV2qoUer8NWVGKJ3whILTvKafVs6U2EL/i4sqJHxrmpE3 1MyJhv9IvKNzjj6E4K0yXE8AVI4bsolk927HcfHjw6STCsTJxfJ0QG3JRiJqv/mRebxW U6nrHwv72FHAo5jP+8Tv8knmotDrJP+RlJpxy5UR4E75Ztyie6UQBJ+ctz22H7ENkHxb ZXHHmu3Y7NTB9rxI1rrKaBULZp6BIGMzcXWHmnT0ArkR0jBjBrT+Zv8u9RVR2Ew3rdu6 NhtHhecYM/KFqypV9COQnK9jvOUjXidzD/RDT7EfHfRiuLmp+EuZvSrWVlh7uBl3DxdX jZgg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=n23TrcYwVq6RNvuD3H1p4j2+Q2TCqgagvpx7ZgA3I9k=; b=PMPPDNo/FI7NiSd/UA0ivy/9dEtprmBsZgpuJ5OM0Xqw3ARsJuDHwMN+eXTyFboqcN saOg0WFazYouG/1yoUzuKZRXJs3hOZcWdLzv11ii9OQ+XVoHds8/z0uz/R9u4fru2EPU 4wp5/GvMJP+tFxNKlrCw+QoW7ioi0ssBsNgb14asNl6HcE+EfUkLSniu81Y47eGLsBEo 0BPVcAM2++AFBpWLzB6wMyWizv/xh568n48+B/DTJ9vFdM33kDENAg2r8JBvIzqthniQ 0CSbxqNQ3KfyT3BS9+B3QQBkuBo9zZ8Gst9qEZ3TnfRHsuUlNxCQIWXkK7W6emmymHuz YFtA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=ZIFTHxHe; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id a129-20020a624d87000000b005a8ad6d343csi12494846pfb.126.2023.03.07.11.35.00; Tue, 07 Mar 2023 11:35:13 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=ZIFTHxHe; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232982AbjCGSiC (ORCPT <rfc822;toshivichauhan@gmail.com> + 99 others); Tue, 7 Mar 2023 13:38:02 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46854 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230096AbjCGShh (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 7 Mar 2023 13:37:37 -0500 Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7D10AA72AB; Tue, 7 Mar 2023 10:29:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1678213744; x=1709749744; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=QLqrVKhYTI6UhfmMR2KhHIzSzpuaBoCKkQO+o6CeEBw=; b=ZIFTHxHeOaaUWNbYD83MO6eMuE2lBFHcI3h0dT6cGdpCsJ20ezA2ek02 jjV88S2YQaEqL5LTeogWp6HBVLCSxRK75d000xCrWfx4p5UtPshrhEV03 GmK6QUN/e59sLfFun6YSwPWAG/Mw0dDSksJNF4LVax8T3k4pHPRD1WsRF fADffrYbm2jdeMTw22bPPHxDm7q0jHPXA4l4rNsGkDWw1FjtuF4JMR0ol XmnP/TiSRRMv67q9z24l69j7tFgLgA9UgEjns8OajtoVaoU06xZx2LKP0 lM1TeVgqJYs7WlxtLfnKqtW49WNT0vSFlsh6D0gOOiJATv8X8JhHCTtVB w==; X-IronPort-AV: E=McAfee;i="6500,9779,10642"; a="316333922" X-IronPort-AV: E=Sophos;i="5.98,241,1673942400"; d="scan'208";a="316333922" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Mar 2023 10:25:17 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10642"; a="706902044" X-IronPort-AV: E=Sophos;i="5.98,241,1673942400"; d="scan'208";a="706902044" Received: from black.fi.intel.com ([10.237.72.28]) by orsmga008.jf.intel.com with ESMTP; 07 Mar 2023 10:25:15 -0800 Received: by black.fi.intel.com (Postfix, from userid 1003) id 1B563354; Tue, 7 Mar 2023 20:25:59 +0200 (EET) From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>, 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> Subject: [PATCH v1 3/3] gpiolib: Move gpiodevice_*() to gpiodev namespace Date: Tue, 7 Mar 2023 20:25:57 +0200 Message-Id: <20230307182557.42215-4-andriy.shevchenko@linux.intel.com> X-Mailer: git-send-email 2.39.1 In-Reply-To: <20230307182557.42215-1-andriy.shevchenko@linux.intel.com> References: <20230307182557.42215-1-andriy.shevchenko@linux.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE, SPF_NONE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1759738817646073070?= X-GMAIL-MSGID: =?utf-8?q?1759738817646073070?= |
Series |
gpiolib: cleanups WRT GPIO device handling
|
|
Commit Message
Andy Shevchenko
March 7, 2023, 6:25 p.m. UTC
The functions that operates on the same device object would
have the same namespace for better code understanding and
maintenance.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/gpio/gpiolib.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Comments
On Tue, Mar 7, 2023 at 7:25 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > The functions that operates on the same device object would > have the same namespace for better code understanding and > maintenance. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/gpio/gpiolib.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index a44a1b0f2766..45f79aee451a 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -573,7 +573,7 @@ bool gpiochip_line_is_valid(const struct gpio_chip *gc, > } > EXPORT_SYMBOL_GPL(gpiochip_line_is_valid); > > -static void gpiodevice_release(struct device *dev) > +static void gpiodev_release(struct device *dev) > { > struct gpio_device *gdev = to_gpio_device(dev); > unsigned long flags; > @@ -617,7 +617,7 @@ static int gpiochip_setup_dev(struct gpio_device *gdev) > return ret; > > /* From this point, the .release() function cleans up gpio_device */ > - gdev->dev.release = gpiodevice_release; > + gdev->dev.release = gpiodev_release; > > ret = gpiochip_sysfs_register(gdev); > if (ret) > -- > 2.39.1 > But the only other function that's in the gpiodev_ namespace operates on struct gpio_device so that change doesn't make much sense to me. Bart
On Wed, Mar 08, 2023 at 11:49:53AM +0100, Bartosz Golaszewski wrote: > On Tue, Mar 7, 2023 at 7:25 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > > The functions that operates on the same device object would > > have the same namespace for better code understanding and > > maintenance. ... > > -static void gpiodevice_release(struct device *dev) > > +static void gpiodev_release(struct device *dev) > > { > > struct gpio_device *gdev = to_gpio_device(dev); > > unsigned long flags; > > @@ -617,7 +617,7 @@ static int gpiochip_setup_dev(struct gpio_device *gdev) > > return ret; > > > > /* From this point, the .release() function cleans up gpio_device */ > > - gdev->dev.release = gpiodevice_release; > > + gdev->dev.release = gpiodev_release; > > > > ret = gpiochip_sysfs_register(gdev); > > if (ret) > But the only other function that's in the gpiodev_ namespace operates > on struct gpio_device so that change doesn't make much sense to me. I'm not sure I understood the comment. After this change we will have static int gpiodev_add_to_list(struct gpio_device *gdev) static void gpiodev_release(struct device *dev) There are also gpio_device_*() I have noticed, so may be these should be actually in that namespace? And we have static int gpiochip_setup_dev(struct gpio_device *gdev) static void gpiolib_dbg_show(struct seq_file *s, struct gpio_device *gdev) That said, what do you think is the best to make this more consistent?
On Thu, Mar 9, 2023 at 7:52 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Wed, Mar 08, 2023 at 11:49:53AM +0100, Bartosz Golaszewski wrote: > > On Tue, Mar 7, 2023 at 7:25 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > > The functions that operates on the same device object would > > > have the same namespace for better code understanding and > > > maintenance. > > ... > > > > -static void gpiodevice_release(struct device *dev) > > > +static void gpiodev_release(struct device *dev) > > > { > > > struct gpio_device *gdev = to_gpio_device(dev); > > > unsigned long flags; > > > @@ -617,7 +617,7 @@ static int gpiochip_setup_dev(struct gpio_device *gdev) > > > return ret; > > > > > > /* From this point, the .release() function cleans up gpio_device */ > > > - gdev->dev.release = gpiodevice_release; > > > + gdev->dev.release = gpiodev_release; > > > > > > ret = gpiochip_sysfs_register(gdev); > > > if (ret) > > > But the only other function that's in the gpiodev_ namespace operates > > on struct gpio_device so that change doesn't make much sense to me. > > I'm not sure I understood the comment. > After this change we will have > > static int gpiodev_add_to_list(struct gpio_device *gdev) > static void gpiodev_release(struct device *dev) > Do you want to use the same prefix for both because struct device in the latter is embedded in struct gpio_device in the former? Bart > There are also gpio_device_*() I have noticed, so may be these should be > actually in that namespace? > > And we have > > static int gpiochip_setup_dev(struct gpio_device *gdev) > static void gpiolib_dbg_show(struct seq_file *s, struct gpio_device *gdev) > > That said, what do you think is the best to make this more consistent? > > -- > With Best Regards, > Andy Shevchenko > >
On Fri, Mar 10, 2023 at 05:48:39PM +0100, Bartosz Golaszewski wrote: > On Thu, Mar 9, 2023 at 7:52 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Wed, Mar 08, 2023 at 11:49:53AM +0100, Bartosz Golaszewski wrote: > > > On Tue, Mar 7, 2023 at 7:25 PM Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > > > > The functions that operates on the same device object would > > > > have the same namespace for better code understanding and > > > > maintenance. ... > > > > -static void gpiodevice_release(struct device *dev) > > > > +static void gpiodev_release(struct device *dev) > > > > { > > > > struct gpio_device *gdev = to_gpio_device(dev); > > > > unsigned long flags; > > > > @@ -617,7 +617,7 @@ static int gpiochip_setup_dev(struct gpio_device *gdev) > > > > return ret; > > > > > > > > /* From this point, the .release() function cleans up gpio_device */ > > > > - gdev->dev.release = gpiodevice_release; > > > > + gdev->dev.release = gpiodev_release; > > > > > > > > ret = gpiochip_sysfs_register(gdev); > > > > if (ret) > > > > > But the only other function that's in the gpiodev_ namespace operates > > > on struct gpio_device so that change doesn't make much sense to me. > > > > I'm not sure I understood the comment. > > After this change we will have > > > > static int gpiodev_add_to_list(struct gpio_device *gdev) > > static void gpiodev_release(struct device *dev) > > > > Do you want to use the same prefix for both because struct device in > the latter is embedded in struct gpio_device in the former? Yes, the logic to have logically grouped namespace for these APIs. Meaning on what they are taking as an effective object to proceed with. > > There are also gpio_device_*() I have noticed, so may be these should be > > actually in that namespace? > > > > And we have > > > > static int gpiochip_setup_dev(struct gpio_device *gdev) > > static void gpiolib_dbg_show(struct seq_file *s, struct gpio_device *gdev) > > > > That said, what do you think is the best to make this more consistent?
On Fri, Mar 10, 2023 at 6:01 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Fri, Mar 10, 2023 at 05:48:39PM +0100, Bartosz Golaszewski wrote: > > On Thu, Mar 9, 2023 at 7:52 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > On Wed, Mar 08, 2023 at 11:49:53AM +0100, Bartosz Golaszewski wrote: > > > > On Tue, Mar 7, 2023 at 7:25 PM Andy Shevchenko > > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > > > > > > The functions that operates on the same device object would > > > > > have the same namespace for better code understanding and > > > > > maintenance. > > ... > > > > > > -static void gpiodevice_release(struct device *dev) > > > > > +static void gpiodev_release(struct device *dev) > > > > > { > > > > > struct gpio_device *gdev = to_gpio_device(dev); > > > > > unsigned long flags; > > > > > @@ -617,7 +617,7 @@ static int gpiochip_setup_dev(struct gpio_device *gdev) > > > > > return ret; > > > > > > > > > > /* From this point, the .release() function cleans up gpio_device */ > > > > > - gdev->dev.release = gpiodevice_release; > > > > > + gdev->dev.release = gpiodev_release; > > > > > > > > > > ret = gpiochip_sysfs_register(gdev); > > > > > if (ret) > > > > > > > But the only other function that's in the gpiodev_ namespace operates > > > > on struct gpio_device so that change doesn't make much sense to me. > > > > > > I'm not sure I understood the comment. > > > After this change we will have > > > > > > static int gpiodev_add_to_list(struct gpio_device *gdev) > > > static void gpiodev_release(struct device *dev) > > > > > > > Do you want to use the same prefix for both because struct device in > > the latter is embedded in struct gpio_device in the former? > > Yes, the logic to have logically grouped namespace for these APIs. > Meaning on what they are taking as an effective object to proceed > with. > I don't have a better idea so applied it. Bart > > > There are also gpio_device_*() I have noticed, so may be these should be > > > actually in that namespace? > > > > > > And we have > > > > > > static int gpiochip_setup_dev(struct gpio_device *gdev) > > > static void gpiolib_dbg_show(struct seq_file *s, struct gpio_device *gdev) > > > > > > That said, what do you think is the best to make this more consistent? > > -- > With Best Regards, > Andy Shevchenko > >
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index a44a1b0f2766..45f79aee451a 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -573,7 +573,7 @@ bool gpiochip_line_is_valid(const struct gpio_chip *gc, } EXPORT_SYMBOL_GPL(gpiochip_line_is_valid); -static void gpiodevice_release(struct device *dev) +static void gpiodev_release(struct device *dev) { struct gpio_device *gdev = to_gpio_device(dev); unsigned long flags; @@ -617,7 +617,7 @@ static int gpiochip_setup_dev(struct gpio_device *gdev) return ret; /* From this point, the .release() function cleans up gpio_device */ - gdev->dev.release = gpiodevice_release; + gdev->dev.release = gpiodev_release; ret = gpiochip_sysfs_register(gdev); if (ret)