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 smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id BA759E6FE2F for ; Fri, 22 Sep 2023 14:14:18 +0000 (UTC) Received: by smtp.kernel.org (Postfix) id 63116C433CC; Fri, 22 Sep 2023 14:14:18 +0000 (UTC) Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.kernel.org (Postfix) with ESMTPS id 234F3C433C7; Fri, 22 Sep 2023 14:14:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 smtp.kernel.org 234F3C433C7 Authentication-Results: smtp.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: smtp.kernel.org; spf=fail smtp.mailfrom=kernel.org X-IronPort-AV: E=McAfee;i="6600,9927,10841"; a="360216622" X-IronPort-AV: E=Sophos;i="6.03,167,1694761200"; d="scan'208";a="360216622" 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?= List-Id: 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 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