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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 46582E6FE35 for ; Fri, 22 Sep 2023 14:14:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234217AbjIVOOc (ORCPT ); Fri, 22 Sep 2023 10:14:32 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55298 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234186AbjIVOO1 (ORCPT ); Fri, 22 Sep 2023 10:14:27 -0400 Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.136]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CAB3FCD8 for ; Fri, 22 Sep 2023 07:14:16 -0700 (PDT) X-IronPort-AV: E=McAfee;i="6600,9927,10841"; a="360216619" X-IronPort-AV: E=Sophos;i="6.03,167,1694761200"; d="scan'208";a="360216619" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Sep 2023 07:14:16 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10841"; a="750859290" X-IronPort-AV: E=Sophos;i="6.03,167,1694761200"; d="scan'208";a="750859290" Received: from smile.fi.intel.com ([10.237.72.54]) by fmsmga007.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Sep 2023 07:14:13 -0700 Received: from andy by smile.fi.intel.com with local (Exim 4.97-RC0) (envelope-from ) id 1qjgv4-0000000HCii-3bDE; Fri, 22 Sep 2023 17:14:10 +0300 Date: Fri, 22 Sep 2023 17:14:10 +0300 From: Andy Shevchenko To: Marek =?iso-8859-1?Q?Beh=FAn?= Cc: Gregory CLEMENT , Arnd Bergmann , soc@kernel.org, arm@kernel.org, Linus Walleij , Bartosz Golaszewski , linux-gpio@vger.kernel.org Subject: Re: [PATCH v2 3/7] platform: cznic: turris-omnia-mcu: Add support for MCU connected GPIOs Message-ID: References: <20230919103815.16818-1-kabel@kernel.org> <20230919103815.16818-4-kabel@kernel.org> <20230921204243.19c48136@thinkpad> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20230921204243.19c48136@thinkpad> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org On Thu, Sep 21, 2023 at 08:42:43PM +0200, Marek Behún wrote: > On Tue, 19 Sep 2023 16:00:39 +0300 > Andy Shevchenko wrote: ... > > > + mutex_lock(&mcu->lock); > > > + > > > + if (ctl_mask) > > > + err = omnia_ctl_cmd_unlocked(mcu, CMD_GENERAL_CONTROL, ctl, > > > + ctl_mask); > > > > > + if (!err && ext_ctl_mask) > > > + err = omnia_ctl_cmd_unlocked(mcu, CMD_EXT_CONTROL, ext_ctl, > > > + ext_ctl_mask); > > > > Can it be > > > > if (err) > > goto out_unlock; > > > > if (_mask) > > ... > > > > ? > > Hi Andy, > > so I am refactoring this to use guard(mutex), but now I have this: > > guard(mutex, &mcu->lock); > > if (ctl_mask) { > err = ...; > if (err) > goto out_err; > } > > if (ext_ctl_mask) { > err = ...; > if (err) > goto out_err; > } > > return; > out_err: > dev_err(dev, "Cannot set GPIOs: %d\n", err); > > which clearly is not any better... or at least the original ...which rather means that the design of above is not so good, i.e. why do you need the same message in the different situations? > if (!err && ext_ctl_mask) > is better IMO. I disagree. > Compare with: > > guard(mutex, &mcu->lock); > > if (ctl_mask) > err = ...; > > if (!err && ext_ctl_mask) > err = ...; > > if (err) > dev_err(dev, "Cannot set GPIOs: %d\n", err); > > > Do you have a better suggestion? Use different messages (if even needed) for different situations. With cleanup.h in place you shouldn't supposed to have goto:s (in simple cases like yours). -- With Best Regards, Andy Shevchenko