Message ID | 79fa4854-976d-4aad-86ac-c156b0c4937e@web.de |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-88021-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7301:2097:b0:108:e6aa:91d0 with SMTP id gs23csp919988dyb; Thu, 29 Feb 2024 23:47:12 -0800 (PST) X-Forwarded-Encrypted: i=3; AJvYcCWZkXVCExREy0/c1ZMlT0Isb9IM5hbD651pvuae/k0OkadQmxT36l2W1m1CnIbE/hmRQvJmNI6U44kVCCRPvkWlo3vCUw== X-Google-Smtp-Source: AGHT+IFSBCZ4uGhdLbx6rhc0ZJgIA5nhnFVSKyt1QZzg9Dil3qYDaq/myTwiCGyyCa8qUQJLkaLg X-Received: by 2002:a50:d6d7:0:b0:566:1952:694c with SMTP id l23-20020a50d6d7000000b005661952694cmr784441edj.20.1709279231824; Thu, 29 Feb 2024 23:47:11 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1709279231; cv=pass; d=google.com; s=arc-20160816; b=ZAV7GY35nA8S5K0rIUUcRGdpqWonO+pRg4JEYGp4MdpisMXaII2Dw0X2SAx/azS5xa KR5xT8CQuuIaWz0P8ekxztaf9r11my/quHssdi0wMaiKZuTjQM5GSFaluASU3OJx3tsY CugaWP5ufHLKGy+6/gAc1ijsaqrMZlnEq1u+L6dBD8v0d96wBglfmRHahVyiBjAXfXud /vG2e7bJVOVZKKGYGGI1UYaXX62cNewPeXRPc+6OxNBvtQc7VlDASpuvpT+QwlpbCT3r /Jgmxzx435CxhksRxSEKs3HNkXmWwtqqRjQ6Yu2rJiAyHOEnvmgyHuh7w2Jcny/v8gfP mIQg== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=ui-outboundreport:content-transfer-encoding:subject:from:cc :content-language:to:user-agent:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:date:message-id:dkim-signature; bh=lS7MyZJFkiU+SRSvzdfTf89X2M03cEzhO12NsTTrgJA=; fh=y7ztJkCg5MNwqqlQ9Vx3zt1X8oTgo8+djqN99nq7+nU=; b=Fum2ntB2G3STKNS88UQkEzAdSvfyJVXIBKHlVn5g9/OtfWXY9LQSiat89Pv4cVRhXo tPWGH1R8v4VlV56NcWL9ofBlVt17ER5rTDd7xhHS1k5ir3+K6Q2s9vswPv3gL6fTylBi xSpTESj29jjc+g9g1N7DCLfiH2tYdIyBKAVcQHlsXUieZC0nmkzytG5rfR7E7oiZsty9 /yw+3AgVdAqT6DpdixE/rQ2zPeHIPEaTgSaA5PpBcvrJjT24CDlfeoctMK9jU9GGfqWA 8ogFPDq+yQ7MI0knPVXytXxuSHSialQv7lyn7ZXGR5f6gdo84vXyZvNG/yipWTRNOj1P kE/A==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@web.de header.s=s29768273 header.b="vEWh/wuj"; arc=pass (i=1 spf=pass spfdomain=web.de dkim=pass dkdomain=web.de dmarc=pass fromdomain=web.de); spf=pass (google.com: domain of linux-kernel+bounces-88021-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) smtp.mailfrom="linux-kernel+bounces-88021-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=web.de Received: from am.mirrors.kernel.org (am.mirrors.kernel.org. [2604:1380:4601:e00::3]) by mx.google.com with ESMTPS id u21-20020a50a415000000b00566a91eda04si1023892edb.229.2024.02.29.23.47.11 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 29 Feb 2024 23:47:11 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-88021-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3; Authentication-Results: mx.google.com; dkim=pass header.i=@web.de header.s=s29768273 header.b="vEWh/wuj"; arc=pass (i=1 spf=pass spfdomain=web.de dkim=pass dkdomain=web.de dmarc=pass fromdomain=web.de); spf=pass (google.com: domain of linux-kernel+bounces-88021-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) smtp.mailfrom="linux-kernel+bounces-88021-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=web.de 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 am.mirrors.kernel.org (Postfix) with ESMTPS id 70A821F21745 for <ouuuleilei@gmail.com>; Fri, 1 Mar 2024 07:47:11 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id E4CBA69D06; Fri, 1 Mar 2024 07:46:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=web.de header.i=markus.elfring@web.de header.b="vEWh/wuj" Received: from mout.web.de (mout.web.de [212.227.15.3]) (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 AE302405DB; Fri, 1 Mar 2024 07:46:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=212.227.15.3 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709279212; cv=none; b=DX9q3YT17fIBif5ZvPGfU6C+FK0X3M5TOESgT15z2b61jR07dRWyYekvoI+w+istQe3hfl8J/uCrGv3NvhPcP2AdEPCqq149n0XgIlXqpN6Jh6AQ6f8/OcOMfEBeT0PR8E/+n+k3AdgOyCTtDSUk3EcnjVGuRaM+tiLiyAZcmGA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709279212; c=relaxed/simple; bh=PtVj2Cs/5utkaUVwbD5AXLYA8GHmi80b51Oqj9vJ8HQ=; h=Message-ID:Date:MIME-Version:To:Cc:From:Subject:Content-Type; b=OVdWBBVUFc4Z3aMPvPJnojWueH3JtSeowaG37lMqRXEsgqL+LlZhtHczykNMNkymyo+znHieSnVpNHEsWgCiOajnSikGLmG4vu8mAx6ThErSSm4KT6UN25jIluDzuaDRQUx2uMhYsJIO0CLprX6FrOIus6xDfhT+paolF4/Wf0c= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=web.de; spf=pass smtp.mailfrom=web.de; dkim=pass (2048-bit key) header.d=web.de header.i=markus.elfring@web.de header.b=vEWh/wuj; arc=none smtp.client-ip=212.227.15.3 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=web.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=web.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=web.de; s=s29768273; t=1709279197; x=1709883997; i=markus.elfring@web.de; bh=PtVj2Cs/5utkaUVwbD5AXLYA8GHmi80b51Oqj9vJ8HQ=; h=X-UI-Sender-Class:Date:To:Cc:From:Subject; b=vEWh/wujuPkr5ZgUKGrC7CR17ajTtwB8K0+7BUgACfosZ+YkVCxOkokrpBZ0MHPL yKIjqDGCfxtom0sTR+kmTUpuj/Ok4KAHTlO0RzySVvafbhY11Ga9PMd5zSS4PyfQk VAX3ElI3F02jdmh7X9DeJJqt3yfJlOnzxr462cjXEc2apqKpTvBXySiaALaGBBoMX I6Hs/BbvcvpBhoCBMaHFPQsQzG05yzM24gN8r+XBlIYbCaArZsEpXpM31FFpOEHOW 7nq8J8PkDaZMkyJ5Fs9eQfpsP6ak3+rXIOvj5AjCv9Rt1xeAiLgmrhFKbnRcpHObg 1uWF8XPffY4vRwqsmg== X-UI-Sender-Class: 814a7b36-bfc1-4dae-8640-3722d8ec6cd6 Received: from [192.168.178.21] ([94.31.86.95]) by smtp.web.de (mrweb006 [213.165.67.108]) with ESMTPSA (Nemesis) id 1MA4fW-1rZx3v2OaY-00Baxt; Fri, 01 Mar 2024 08:46:37 +0100 Message-ID: <79fa4854-976d-4aad-86ac-c156b0c4937e@web.de> Date: Fri, 1 Mar 2024 08:46:25 +0100 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 User-Agent: Mozilla Thunderbird To: linux-media@vger.kernel.org, kernel-janitors@vger.kernel.org, Andy Shevchenko <andriy.shevchenko@linux.intel.com>, Mauro Carvalho Chehab <mchehab@kernel.org>, Sakari Ailus <sakari.ailus@linux.intel.com>, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> Content-Language: en-GB Cc: LKML <linux-kernel@vger.kernel.org> From: Markus Elfring <Markus.Elfring@web.de> Subject: [PATCH] media: i2c: ds90ub960: Delete duplicate source code in ub960_parse_dt_rxports() Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Provags-ID: V03:K1:NG6uo48tDikPs/0i4tEmGu816A1pQ83tsl2OXdg3u6hJ5bwtoyO BjRU6fcWGkrgTk6joa1FlnF7RBUozfSJH9fqxJz0A/9TmzxZpXJTDatt1LM6B3NsqdN5T26 H853kokKn63cWdDZhBzT1ojYkPPy/rDan4daCVEou18V1+O5cXYY3P5ds0+ijVcTx9+xs+7 SXRSO+1JjmgDt/TNkOLDg== X-Spam-Flag: NO UI-OutboundReport: notjunk:1;M01:P0:Z/ukpIoRDls=;0SinomGpMNGuyZ7CuXAkKrUBS69 o5VCs//WXLxsJQAdvYJT6ILPRcp+zspbC2T8zbwkUYBbMK7VQH3YrN1nXJQ7lz+VP7qB5HCQ7 n0/2lrwFfPwLCLSAx2nobiKHpwod+w+dPsLaqJzvpckXRLU5x/WZHQw8HfqkC//80zFd4fxv4 XMlFzDr5n5CC5os48AgQIe2jg3wPcH7gho1UcMg5RHfDD4Z/MvyJFkNbjYDXbbXsrBUH1jkiH Gvyptdpm1ftYOsc7TuH29vWpSWTqFGQ4n3NjkcP4U/OvWq3nDC1bdDLPUxj+yHN4uEYniMw36 mCf0odVkbJ7ubKuBmdM5ZymzMNYfVlmvEhsYDOnLRL8Z0LYWwUX1Imd82WWD/fTVVKBsAtGQ+ rT+tnd3um6Eag2RfK72cIPA4hzeD+14/ErRAMKAEgkJ9DaoHdCN664tBqak+n880rt8zZa4Xj X1HJb56WmFXbVpuBjcX+Fv/NfPHefUHFK4wyeYYwCO01I+8qYdOBKHpo8VWzAxpJuzqTBdtJD 8ZMn7OihOAHohXt1H9rzkeX0ezhzA50MA85RQSCycfQWd670621IiHhqprNTZy3VheNbNId6K 5d5Kzl9TpRWKAFFUE1bADesH6ffb5BGJCL4QUeltuGPotcTwUJN+8ubtHccdSHRegVDgsLMjG s7I8lfaJh900ZealoffrLoTp7e/d86mNk1KU+MSaeaDgpEy+lZKUaXeZrTd4zR5ztMeBQYdYA RcV5FisSS9OjRwARJpVix0Gw3k6Stevs/69UqHG9kOqsiTpD+3yuopk1UjOG46JfjaFDJvehl xxs6FmrjLYbbqpC7/0WYXaGaboDF5hmko5g4g/ePy0tCw= X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1792309179916969812 X-GMAIL-MSGID: 1792309179916969812 |
Series |
media: i2c: ds90ub960: Delete duplicate source code in ub960_parse_dt_rxports()
|
|
Commit Message
Markus Elfring
March 1, 2024, 7:46 a.m. UTC
From: Markus Elfring <elfring@users.sourceforge.net> Date: Fri, 1 Mar 2024 08:23:24 +0100 Avoid the specification of a duplicate fwnode_handle_put() call in this function implementation. Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> --- drivers/media/i2c/ds90ub960.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) -- 2.44.0
Comments
Hi Markus, On Fri, Mar 01, 2024 at 08:46:25AM +0100, Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Fri, 1 Mar 2024 08:23:24 +0100 > > Avoid the specification of a duplicate fwnode_handle_put() call > in this function implementation. > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > --- > drivers/media/i2c/ds90ub960.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c > index ffe5f25f8647..eb708ed7b56e 100644 > --- a/drivers/media/i2c/ds90ub960.c > +++ b/drivers/media/i2c/ds90ub960.c > @@ -3486,10 +3486,7 @@ static int ub960_parse_dt_rxports(struct ub960_data *priv) > } > } > > - fwnode_handle_put(links_fwnode); > - > - return 0; > - > + ret = 0; I think it'd be nicer to initialise ret as zero, then you can just drop the assignment above. > err_put_links: > fwnode_handle_put(links_fwnode); >
Hi, On 01/03/2024 10:46, Sakari Ailus wrote: > Hi Markus, > > On Fri, Mar 01, 2024 at 08:46:25AM +0100, Markus Elfring wrote: >> From: Markus Elfring <elfring@users.sourceforge.net> >> Date: Fri, 1 Mar 2024 08:23:24 +0100 >> >> Avoid the specification of a duplicate fwnode_handle_put() call >> in this function implementation. >> >> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> >> --- >> drivers/media/i2c/ds90ub960.c | 5 +---- >> 1 file changed, 1 insertion(+), 4 deletions(-) >> >> diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c >> index ffe5f25f8647..eb708ed7b56e 100644 >> --- a/drivers/media/i2c/ds90ub960.c >> +++ b/drivers/media/i2c/ds90ub960.c >> @@ -3486,10 +3486,7 @@ static int ub960_parse_dt_rxports(struct ub960_data *priv) >> } >> } >> >> - fwnode_handle_put(links_fwnode); >> - >> - return 0; >> - >> + ret = 0; > > I think it'd be nicer to initialise ret as zero, then you can just drop the > assignment above. I don't like successful execution entering error paths. That's why there's the return 0. Tomi >> err_put_links: >> fwnode_handle_put(links_fwnode); >> >
Huomenta, On Fri, Mar 01, 2024 at 10:49:19AM +0200, Tomi Valkeinen wrote: > Hi, > > On 01/03/2024 10:46, Sakari Ailus wrote: > > Hi Markus, > > > > On Fri, Mar 01, 2024 at 08:46:25AM +0100, Markus Elfring wrote: > > > From: Markus Elfring <elfring@users.sourceforge.net> > > > Date: Fri, 1 Mar 2024 08:23:24 +0100 > > > > > > Avoid the specification of a duplicate fwnode_handle_put() call > > > in this function implementation. > > > > > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > > > --- > > > drivers/media/i2c/ds90ub960.c | 5 +---- > > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > > > diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c > > > index ffe5f25f8647..eb708ed7b56e 100644 > > > --- a/drivers/media/i2c/ds90ub960.c > > > +++ b/drivers/media/i2c/ds90ub960.c > > > @@ -3486,10 +3486,7 @@ static int ub960_parse_dt_rxports(struct ub960_data *priv) > > > } > > > } > > > > > > - fwnode_handle_put(links_fwnode); > > > - > > > - return 0; > > > - > > > + ret = 0; > > > > I think it'd be nicer to initialise ret as zero, then you can just drop the > > assignment above. > > I don't like successful execution entering error paths. That's why there's > the return 0. It could be called a common cleanup path as what you really want to do here is to put the fwnode handle, independently of whether there was an error. I think the current code is of course fine, too. Soon you can do struct fwnode_handle *links_fwnode __free(fwnode_handle); and forget about putting it (but you must need putting it).
On Fri, Mar 01, 2024 at 09:02:41AM +0000, Sakari Ailus wrote: > On Fri, Mar 01, 2024 at 10:49:19AM +0200, Tomi Valkeinen wrote: > > On 01/03/2024 10:46, Sakari Ailus wrote: > > > On Fri, Mar 01, 2024 at 08:46:25AM +0100, Markus Elfring wrote: > > > > From: Markus Elfring <elfring@users.sourceforge.net> > > > > Date: Fri, 1 Mar 2024 08:23:24 +0100 > > > > > > > > Avoid the specification of a duplicate fwnode_handle_put() call > > > > in this function implementation. > > > > > > > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > > > > --- > > > > drivers/media/i2c/ds90ub960.c | 5 +---- > > > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > > > > > diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c > > > > index ffe5f25f8647..eb708ed7b56e 100644 > > > > --- a/drivers/media/i2c/ds90ub960.c > > > > +++ b/drivers/media/i2c/ds90ub960.c > > > > @@ -3486,10 +3486,7 @@ static int ub960_parse_dt_rxports(struct ub960_data *priv) > > > > } > > > > } > > > > > > > > - fwnode_handle_put(links_fwnode); > > > > - > > > > - return 0; > > > > - > > > > + ret = 0; > > > > > > I think it'd be nicer to initialise ret as zero, then you can just drop the > > > assignment above. I think tearing apart the assignment and its actual user is not good. > > I don't like successful execution entering error paths. That's why there's > > the return 0. > > It could be called a common cleanup path as what you really want to do here > is to put the fwnode handle, independently of whether there was an error. > I think the current code is of course fine, too. > > Soon you can do > > struct fwnode_handle *links_fwnode __free(fwnode_handle); > > and forget about putting it (but you must need putting it). Let's wait for the Jonathan's patches to land (v6.9-rc1 I hope) and then we may modify drivers if needed.
On Fri, Mar 01, 2024 at 07:36:17PM +0200, Andy Shevchenko wrote: > On Fri, Mar 01, 2024 at 09:02:41AM +0000, Sakari Ailus wrote: > > On Fri, Mar 01, 2024 at 10:49:19AM +0200, Tomi Valkeinen wrote: > > > On 01/03/2024 10:46, Sakari Ailus wrote: > > > > On Fri, Mar 01, 2024 at 08:46:25AM +0100, Markus Elfring wrote: > > > > > From: Markus Elfring <elfring@users.sourceforge.net> > > > > > Date: Fri, 1 Mar 2024 08:23:24 +0100 > > > > > > > > > > Avoid the specification of a duplicate fwnode_handle_put() call > > > > > in this function implementation. > > > > > > > > > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > > > > > --- > > > > > drivers/media/i2c/ds90ub960.c | 5 +---- > > > > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > > > > > > > diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c > > > > > index ffe5f25f8647..eb708ed7b56e 100644 > > > > > --- a/drivers/media/i2c/ds90ub960.c > > > > > +++ b/drivers/media/i2c/ds90ub960.c > > > > > @@ -3486,10 +3486,7 @@ static int ub960_parse_dt_rxports(struct ub960_data *priv) > > > > > } > > > > > } > > > > > > > > > > - fwnode_handle_put(links_fwnode); > > > > > - > > > > > - return 0; > > > > > - > > > > > + ret = 0; > > > > > > > > I think it'd be nicer to initialise ret as zero, then you can just drop the > > > > assignment above. > > I think tearing apart the assignment and its actual user is not good. > > > > I don't like successful execution entering error paths. That's why there's > > > the return 0. > > > > It could be called a common cleanup path as what you really want to do here > > is to put the fwnode handle, independently of whether there was an error. > > I think the current code is of course fine, too. > > > > Soon you can do > > > > struct fwnode_handle *links_fwnode __free(fwnode_handle); > > > > and forget about putting it (but you must need putting it). > > Let's wait for the Jonathan's patches to land (v6.9-rc1 I hope) and then > we may modify drivers if needed. The __free(fwnode_handle) stuff has already been merged. We could do some additional work to make a _scoped() macro for fwnode_handles but here it's function wide so we already have what we need. regards, dan carpenter
> The __free(fwnode_handle) stuff has already been merged.
Would you like to point a corresponding commit out?
Regards,
Markus
diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c index ffe5f25f8647..eb708ed7b56e 100644 --- a/drivers/media/i2c/ds90ub960.c +++ b/drivers/media/i2c/ds90ub960.c @@ -3486,10 +3486,7 @@ static int ub960_parse_dt_rxports(struct ub960_data *priv) } } - fwnode_handle_put(links_fwnode); - - return 0; - + ret = 0; err_put_links: fwnode_handle_put(links_fwnode);