Message ID | 20240227175910.3031342-2-andriy.shevchenko@linux.intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-83789-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:a81b:b0:108:e6aa:91d0 with SMTP id bq27csp2946337dyb; Tue, 27 Feb 2024 12:25:17 -0800 (PST) X-Forwarded-Encrypted: i=3; AJvYcCUIdbByINLAc5L/pmeJ+/Ej5NeINQMeO1HGphvqlKeF68GqWAS2tyKjFKWpAs47KVgSpNclokECuY9K5hHs5NL6kiT+bg== X-Google-Smtp-Source: AGHT+IHTEwr75dNzTFVlXA4uL3u+1kKD9r3Rd/1RbaLOJg8hXY2/LMA/x4lYukH41x6dD+lep1yp X-Received: by 2002:a05:6808:1287:b0:3c1:4109:d147 with SMTP id a7-20020a056808128700b003c14109d147mr3630440oiw.9.1709065517386; Tue, 27 Feb 2024 12:25:17 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1709065517; cv=pass; d=google.com; s=arc-20160816; b=bVF9EaTlJIbaxWLAd9TuiE17Qztvkn2WhsXB7rQVY0TqkQ4gz3UbHcF3Z08mB4gZT+ tC7JNriPS0LrrlMJU8Q+gySu11vhorKpkfTXzFX3PMyyD9W6FfYzq9A6yU0nPgPlduiT 2Idac0JrH67Zez8hWDYc1BIEotEWgFpwcBDzR7SwT/pkGE6zfqhbG1aJ8zq8qpBMR8/F slT8lgPONEM5rpI89YWXlY9RqmgTD6WbK+qTjM9EJCRXmvb4Qojc3vNUmC6Inac0Xo+0 xp/nduBpU04W6hKzZAVmHU64VoovDyW7K8d9jUXHgWiYe0f8w3ycam0l9wbT8X9w75Jz UaqQ== ARC-Message-Signature: i=2; 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:references:in-reply-to:message-id :date:subject:cc:to:from:dkim-signature; bh=w38yf0lJCTTrEoxLd+klVT9j+C7X88qmyV8M2MTuOek=; fh=GpQb4kIYKqYHgUivqzizA5C6V4pMAaMxsFnDU4HP8GA=; b=Db/1E5NP2ew/VNzMp8Ni0D4W73LrFZquPlODQCm+X9Th7z0ZcBQuJdn4MVPYFkjrn8 h22/0VH5dlH1NtZZ5J/xcFDFvmMgJlItBcH8Rmh2igtpP6XGfbWrVEwzKn59J86sUx8V yZ8LuzPBPzszbQzRW5cBUWB0I2mdmsDfcvsKQQ1swRYAGb4cQoKYz0rhJQeVjRCOI9Rb zXrasE0R20JWX06plh35cx3PLJqhp4bvXpUX3M9344mD2Aa/NW0vUTdrRhxDrNyRfK/i teFkYz0QcfqFf3zNoe5ESDIYDNgsiOpDoZcQWpdq5yTY2h0YQEb+ImPF8WHG8Qv8R1sH Ydhw==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=g3AIRstB; arc=pass (i=1 dkim=pass dkdomain=intel.com dmarc=pass fromdomain=linux.intel.com); spf=pass (google.com: domain of linux-kernel+bounces-83789-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-83789-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [2604:1380:45e3:2400::1]) by mx.google.com with ESMTPS id r14-20020a63514e000000b005dc4328dc23si5963754pgl.780.2024.02.27.12.25.17 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 27 Feb 2024 12:25:17 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-83789-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=g3AIRstB; arc=pass (i=1 dkim=pass dkdomain=intel.com dmarc=pass fromdomain=linux.intel.com); spf=pass (google.com: domain of linux-kernel+bounces-83789-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-83789-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 sv.mirrors.kernel.org (Postfix) with ESMTPS id 56CC7285042 for <ouuuleilei@gmail.com>; Tue, 27 Feb 2024 17:59:55 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 4999E14900D; Tue, 27 Feb 2024 17:59:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="g3AIRstB" Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.8]) (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 058634EB46 for <linux-kernel@vger.kernel.org>; Tue, 27 Feb 2024 17:59:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.8 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709056763; cv=none; b=aubSFgBaiypqkDHIJvomIozE55/iSxQvayDK6fL4Gtr+BqY8wCkmfTT8OyLLh3seZKDCioPlGPkVfGHTtKGf9gY4lRSTryN5URQV7ENyRNjixt+E1shQXzj4ttKw7k5uuXS3lN2dAtztyF5WJpMhVSHTPcfALPZmPiOLPwaF+iU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709056763; c=relaxed/simple; bh=S7gy7CPHZvEztwdvm1D8suyt5zFSEhNklWiuAuwuQws=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=ZFlUnGNpr5De7w/f4oDtAdsLdkI2kHClIT44NBuir+c/QiUIZqe87WmP3wZ2F7PXDUSwomM9A3K5u7YnOycoS2HqbMyQxICGuCCFlUhvtfp5XQoSKLxEjt85nSISZuG8J2eqnt9agH9KJkEHngcvuV7FjbL2J1qmT2xgzrO+nzg= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com; spf=none smtp.mailfrom=linux.intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=g3AIRstB; arc=none smtp.client-ip=192.198.163.8 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=1709056762; x=1740592762; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=S7gy7CPHZvEztwdvm1D8suyt5zFSEhNklWiuAuwuQws=; b=g3AIRstB5Cn6n27Xu5Y7LUJYBLwWmTGiguj8wndG6iZGGXXjEJs9lrQb s6dwDtgBRGTOw4oyMVtBAXzN4xISHA0BCa/RaVN04DSJaognaSEetKTBv a8X+aNxVLaNPw0u1vXEJR8eROZb2xSGF9t5kS9BDZxXwqbtrZqg6BLlGq Wtegiv1ggcgP1pA+mQMWy27zpav0U7viBvWve7sTy2ExaMuRaS3zamEk5 B8mF/RJEZc3P6xWrSvqTVv3oLDQy9QZJf+u93JbgNKFhdWAemU9biZmDe VwDitPwUh60OUywERN6aqr/wp3VpuFrFtXwrl8OFn9CLp90KFLgpeGy5E A==; X-IronPort-AV: E=McAfee;i="6600,9927,10996"; a="20962782" X-IronPort-AV: E=Sophos;i="6.06,188,1705392000"; d="scan'208";a="20962782" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmvoesa102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Feb 2024 09:59:19 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10996"; a="937032680" X-IronPort-AV: E=Sophos;i="6.06,188,1705392000"; d="scan'208";a="937032680" Received: from black.fi.intel.com ([10.237.72.28]) by fmsmga001.fm.intel.com with ESMTP; 27 Feb 2024 09:59:17 -0800 Received: by black.fi.intel.com (Postfix, from userid 1003) id 0A4B5192; Tue, 27 Feb 2024 19:59:16 +0200 (EET) From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>, Philipp Stanner <pstanner@redhat.com>, linux-kernel@vger.kernel.org Cc: Andrew Morton <akpm@linux-foundation.org>, Rasmus Villemoes <linux@rasmusvillemoes.dk> Subject: [PATCH v1 1/2] devres: Switch to use dev_err_probe() for unification Date: Tue, 27 Feb 2024 19:58:34 +0200 Message-ID: <20240227175910.3031342-2-andriy.shevchenko@linux.intel.com> X-Mailer: git-send-email 2.43.0.rc1.1.gbec44491f096 In-Reply-To: <20240227175910.3031342-1-andriy.shevchenko@linux.intel.com> References: <20240227175910.3031342-1-andriy.shevchenko@linux.intel.com> 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: 1792085083796498859 X-GMAIL-MSGID: 1792085083796498859 |
Series |
devres: A couple of cleanups
|
|
Commit Message
Andy Shevchenko
Feb. 27, 2024, 5:58 p.m. UTC
The devm_*() APIs are supposed to be called during the ->probe() stage.
Many drivers (especially new ones) has switched to use dev_err_probe()
for error messaging for the sake of unification. Let's do the same in
the devres APIs.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
lib/devres.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
Comments
On Tue, 2024-02-27 at 19:58 +0200, Andy Shevchenko wrote: > The devm_*() APIs are supposed to be called during the ->probe() > stage. > Many drivers (especially new ones) has switched to use has -> have > dev_err_probe() > for error messaging for the sake of unification. Let's do the same in > the devres APIs. No objections on principle. Just one thing about the implementation: > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > lib/devres.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/lib/devres.c b/lib/devres.c > index fe0c63caeb68..27f280a39dca 100644 > --- a/lib/devres.c > +++ b/lib/devres.c > @@ -125,12 +125,13 @@ __devm_ioremap_resource(struct device *dev, > const struct resource *res, > resource_size_t size; > void __iomem *dest_ptr; > char *pretty_name; > + int ret; > > BUG_ON(!dev); > > if (!res || resource_type(res) != IORESOURCE_MEM) { > - dev_err(dev, "invalid resource %pR\n", res); > - return IOMEM_ERR_PTR(-EINVAL); > + ret = dev_err_probe(dev, -EINVAL, "invalid resource > %pR\n", res); > + return IOMEM_ERR_PTR(ret); So as I see it -EINVAL is just piped through dev_err_probe() and is never changed. Don't you think it would be better to drop variable 'ret' and just do return IOMEM_ERR_PTR(-EINVAL); as before? That way it would be obvious that the error code is never changed and it will always return -EINVAL. Otherwise you have to look up the function definition of dev_err_probe(). The same would apply below. Regards, P. > } > > if (type == DEVM_IOREMAP && res->flags & > IORESOURCE_MEM_NONPOSTED) > @@ -144,20 +145,20 @@ __devm_ioremap_resource(struct device *dev, > const struct resource *res, > else > pretty_name = devm_kstrdup(dev, dev_name(dev), > GFP_KERNEL); > if (!pretty_name) { > - dev_err(dev, "can't generate pretty name for resource > %pR\n", res); > - return IOMEM_ERR_PTR(-ENOMEM); > + ret = dev_err_probe(dev, -ENOMEM, "can't generate > pretty name for resource %pR\n", res); > + return IOMEM_ERR_PTR(ret); > } > > if (!devm_request_mem_region(dev, res->start, size, > pretty_name)) { > - dev_err(dev, "can't request region for resource > %pR\n", res); > - return IOMEM_ERR_PTR(-EBUSY); > + ret = dev_err_probe(dev, -EBUSY, "can't request > region for resource %pR\n", res); > + return IOMEM_ERR_PTR(ret); > } > > dest_ptr = __devm_ioremap(dev, res->start, size, type); > if (!dest_ptr) { > - dev_err(dev, "ioremap failed for resource %pR\n", > res); > devm_release_mem_region(dev, res->start, size); > - dest_ptr = IOMEM_ERR_PTR(-ENOMEM); > + ret = dev_err_probe(dev, -ENOMEM, "ioremap failed for > resource %pR\n", res); > + return IOMEM_ERR_PTR(ret); > } > > return dest_ptr;
On Thu, Feb 29, 2024 at 09:52:34AM +0100, Philipp Stanner wrote: > On Tue, 2024-02-27 at 19:58 +0200, Andy Shevchenko wrote: > > The devm_*() APIs are supposed to be called during the ->probe() > > stage. > > Many drivers (especially new ones) has switched to use > > has -> have Thanks, will fix. > > dev_err_probe() > > for error messaging for the sake of unification. Let's do the same in > > the devres APIs. > > No objections on principle. Just one thing about the implementation: .. > > + ret = dev_err_probe(dev, -EINVAL, "invalid resource > > %pR\n", res); > > + return IOMEM_ERR_PTR(ret); > > So as I see it -EINVAL is just piped through dev_err_probe() and is > never changed. > Don't you think it would be better to drop variable 'ret' and just do > return IOMEM_ERR_PTR(-EINVAL); > as before? dev_err_probe() requires error code as a parameter. Are you suggesting to have a duplication? dev_err_probe(-EINVAL); return -EINVAL; I don't think it's a good suggestion, so the answer is "No, I don't think it would be better." > That way it would be obvious that the error code is never changed and > it will always return -EINVAL. Otherwise you have to look up the > function definition of dev_err_probe(). .. > The same would apply below. Same answer.
On Thu, Feb 29, 2024 at 12:39:55PM +0200, Andy Shevchenko wrote: > On Thu, Feb 29, 2024 at 09:52:34AM +0100, Philipp Stanner wrote: > > On Tue, 2024-02-27 at 19:58 +0200, Andy Shevchenko wrote: .. > > That way it would be obvious that the error code is never changed and > > it will always return -EINVAL. Otherwise you have to look up the > > function definition of dev_err_probe(). And to this, it's a common pattern in some cases, esp. when you have a chain of the similar calls and you want to simplify the code (see CCI accessors in Video4Linux2 as an example). So, I also don't think it's something unusual.
diff --git a/lib/devres.c b/lib/devres.c index fe0c63caeb68..27f280a39dca 100644 --- a/lib/devres.c +++ b/lib/devres.c @@ -125,12 +125,13 @@ __devm_ioremap_resource(struct device *dev, const struct resource *res, resource_size_t size; void __iomem *dest_ptr; char *pretty_name; + int ret; BUG_ON(!dev); if (!res || resource_type(res) != IORESOURCE_MEM) { - dev_err(dev, "invalid resource %pR\n", res); - return IOMEM_ERR_PTR(-EINVAL); + ret = dev_err_probe(dev, -EINVAL, "invalid resource %pR\n", res); + return IOMEM_ERR_PTR(ret); } if (type == DEVM_IOREMAP && res->flags & IORESOURCE_MEM_NONPOSTED) @@ -144,20 +145,20 @@ __devm_ioremap_resource(struct device *dev, const struct resource *res, else pretty_name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL); if (!pretty_name) { - dev_err(dev, "can't generate pretty name for resource %pR\n", res); - return IOMEM_ERR_PTR(-ENOMEM); + ret = dev_err_probe(dev, -ENOMEM, "can't generate pretty name for resource %pR\n", res); + return IOMEM_ERR_PTR(ret); } if (!devm_request_mem_region(dev, res->start, size, pretty_name)) { - dev_err(dev, "can't request region for resource %pR\n", res); - return IOMEM_ERR_PTR(-EBUSY); + ret = dev_err_probe(dev, -EBUSY, "can't request region for resource %pR\n", res); + return IOMEM_ERR_PTR(ret); } dest_ptr = __devm_ioremap(dev, res->start, size, type); if (!dest_ptr) { - dev_err(dev, "ioremap failed for resource %pR\n", res); devm_release_mem_region(dev, res->start, size); - dest_ptr = IOMEM_ERR_PTR(-ENOMEM); + ret = dev_err_probe(dev, -ENOMEM, "ioremap failed for resource %pR\n", res); + return IOMEM_ERR_PTR(ret); } return dest_ptr;