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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 9D9FCC021AA for ; Wed, 19 Feb 2025 13:21:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Idpx6sjct0DDTZdoqT5ZdFwAJLz5Gc2V5caoYkF+hWM=; b=O97YDm8O+Dz8/TmQiahdWSSwQv BqIxYDdHf6IJDDS2XcLJfi+4LskLO6jxcrQw7lRbCzA/qtifc/+JTtOW3wFndtKuP1L7F7yqakMRI 8JElS0PkJ0BQoK0fsc7gGW97jt6VQt2ay9Z/ZJzj6bjvKch3kTVtpwXXaGWGdr7l4XCtIRGTefliH Sn8W2MjofmJ2mt/dszxQwc2QAjKyWW1CA2mNjndgGjQLKEhEaQ6ZaWe6//AKStQPP3IupOXuKJaYq FFHSm4KyqE1QZMi4M7hykyj67FG5wxYAh6wSkGYFoJdFXSfwcMTJfhZKtTyp2zA3Bz4Uj8rnFWcgQ N+/14giA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1tkk0a-0000000Cuvp-2iXa; Wed, 19 Feb 2025 13:21:00 +0000 Received: from nyc.source.kernel.org ([147.75.193.91]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1tkjz6-0000000CuXt-2hCm; Wed, 19 Feb 2025 13:19:29 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 39EF6A426F9; Wed, 19 Feb 2025 13:17:42 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 26083C4CED1; Wed, 19 Feb 2025 13:19:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1739971166; bh=RcuQPWBAOebggXpc49oXCroXSGfFXw0R3wUoVFgSYw0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=TZH/bQk4qL6+2tnUJqqWBkPPTthw5gu9C17UymFAIupjP536vP5/A2uuLTBTsi5EM G6Cwv45NY3dyN2GbwkOU/ec7M/aKurvYZOZohZZHB3xEdjNwkIGxGcUW3qgOJrXUR0 jXyHDhQTIbUsWIurk+PCF0wp5cC3JQAlgnZOUB20= Date: Wed, 19 Feb 2025 14:19:23 +0100 From: Greg Kroah-Hartman To: Dmitry Baryshkov Cc: Jerome Brunet , Dave Ertman , Ira Weiny , "Rafael J. Wysocki" , Stephen Boyd , Arnd Bergmann , Danilo Krummrich , Conor Dooley , Daire McNamara , Philipp Zabel , Douglas Anderson , Andrzej Hajda , Neil Armstrong , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Hans de Goede , Ilpo =?iso-8859-1?Q?J=E4rvinen?= , Bryan O'Donoghue , Vladimir Kondratiev , Gregory CLEMENT , =?iso-8859-1?Q?Th=E9o?= Lebrun , Michael Turquette , Abel Vesa , Peng Fan , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , Kevin Hilman , Martin Blumenstingl , linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org, dri-devel@lists.freedesktop.org, platform-driver-x86@vger.kernel.org, linux-mips@vger.kernel.org, linux-clk@vger.kernel.org, imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org, linux-amlogic@lists.infradead.org Subject: Re: [PATCH v4 1/8] driver core: auxiliary bus: add device creation helpers Message-ID: <2025021937-trodden-snowdrop-99be@gregkh> References: <20250218-aux-device-create-helper-v4-0-c3d7dfdea2e6@baylibre.com> <20250218-aux-device-create-helper-v4-1-c3d7dfdea2e6@baylibre.com> <2025021922-spongy-swirl-0746@gregkh> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250219_051928_831487_97F18C9B X-CRM114-Status: GOOD ( 31.21 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Feb 19, 2025 at 02:08:22PM +0200, Dmitry Baryshkov wrote: > On Wed, Feb 19, 2025 at 11:13:14AM +0100, Greg Kroah-Hartman wrote: > > On Wed, Feb 19, 2025 at 11:06:02AM +0200, Dmitry Baryshkov wrote: > > > On Tue, Feb 18, 2025 at 08:29:46PM +0100, Jerome Brunet wrote: > > > > Add helper functions to create a device on the auxiliary bus. > > > > > > > > This is meant for fairly simple usage of the auxiliary bus, to avoid having > > > > the same code repeated in the different drivers. > > > > > > > > Suggested-by: Stephen Boyd > > > > Cc: Arnd Bergmann > > > > Signed-off-by: Jerome Brunet > > > > --- > > > > drivers/base/auxiliary.c | 108 ++++++++++++++++++++++++++++++++++++++++++ > > > > include/linux/auxiliary_bus.h | 17 +++++++ > > > > 2 files changed, 125 insertions(+) > > > > > > > > diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c > > > > index afa4df4c5a3f371b91d8dd8c4325495d32ad1291..a6d46c2759be81a0739f07528d5959c2a76eb8a8 100644 > > > > --- a/drivers/base/auxiliary.c > > > > +++ b/drivers/base/auxiliary.c > > > > @@ -385,6 +385,114 @@ void auxiliary_driver_unregister(struct auxiliary_driver *auxdrv) > > > > } > > > > EXPORT_SYMBOL_GPL(auxiliary_driver_unregister); > > > > > > > > +static void auxiliary_device_release(struct device *dev) > > > > +{ > > > > + struct auxiliary_device *auxdev = to_auxiliary_dev(dev); > > > > + > > > > + kfree(auxdev); > > > > +} > > > > + > > > > +/** > > > > + * auxiliary_device_create - create a device on the auxiliary bus > > > > + * @dev: parent device > > > > + * @modname: module name used to create the auxiliary driver name. > > > > + * @devname: auxiliary bus device name > > > > + * @platform_data: auxiliary bus device platform data > > > > + * @id: auxiliary bus device id > > > > + * > > > > + * Helper to create an auxiliary bus device. > > > > + * The device created matches driver 'modname.devname' on the auxiliary bus. > > > > + */ > > > > +struct auxiliary_device *auxiliary_device_create(struct device *dev, > > > > + const char *modname, > > > > + const char *devname, > > > > + void *platform_data, > > > > + int id) > > > > +{ > > > > + struct auxiliary_device *auxdev; > > > > + int ret; > > > > + > > > > + auxdev = kzalloc(sizeof(*auxdev), GFP_KERNEL); > > > > + if (!auxdev) > > > > + return NULL; > > > > + > > > > + auxdev->id = id; > > > > + auxdev->name = devname; > > > > + auxdev->dev.parent = dev; > > > > + auxdev->dev.platform_data = platform_data; > > > > + auxdev->dev.release = auxiliary_device_release; > > > > + device_set_of_node_from_dev(&auxdev->dev, dev); > > > > + > > > > + ret = auxiliary_device_init(auxdev); > > > > + if (ret) { > > > > + kfree(auxdev); > > > > + return NULL; > > > > + } > > > > + > > > > + ret = __auxiliary_device_add(auxdev, modname); > > > > + if (ret) { > > > > > > This loses possible error return values from __auxiliary_device_add(). > > > > Why does that really matter? > > At the very least the caller (or caller of a caller) can call > dev_err_probe() or dev_err("%pe"). With the current implementation as > everybody maps NULL to -ENOMEM the error message will be cryptic. > > Or just having a cryptic value in the logs. So all you can get here could be: -ENOMEM - memory couldn't be allocated somewhere -EINVAL - wrong parameters sent to auxiliary_device_init() or __auxiliary_device_add() -EEXIST - duplicate name And if -EEXIST happens, you will get a kernel log splat from sysfs showing you that something went wrong. So while I understand the need to be specific here in reporting the exact error, I fail to understand how it really matters at all. A driver writer really only wants to know "did it work?" and have a simple way to test it. IS_ERR_OR_NULL() and then getting the error using PTR_ERR() is rough and feels like boilerplate code that everyone gets wrong (how many times do people accidentally only check for NULL?). Anyway, I'm for simple apis, and NULL or valid pointer seems simple to me. thanks, greg k-h