From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ej1-f49.google.com (mail-ej1-f49.google.com [209.85.218.49]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D9B873E1CF7 for ; Tue, 5 May 2026 07:31:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.49 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777966310; cv=none; b=L8FvwMMYP7wkvGkOKjBCneXqn48LLQPImz3CoQA6t7LVVs4fHhLXjcUSNhdnaI2OKdEqLYQ7DemvHopSaAcZ8Gm598hnU1QtptCRNCMPD5ND3yPxLTc/Jd8ToX8hNk/mCQqx7yd4R7/08Sxa+RS12sC7k/7Gdr4k2j5DyEQdFMM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777966310; c=relaxed/simple; bh=TraXVe+N/S65fWQbItJa+1pkQNne+jqlvIwTlL7GSjc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=LY7aXPtCir0RstwWDuT++7epWaHnz8jab8LOPGS8fHMiAPnOcl4i/X3loM0Z3BNcOB1yfJWmRM+mqoHgSiIhn1vKdqwODjkTyvX4lGN+groMi2omwoKWJGuqUs+OXkVlmD8Jx0BhGwiTm9KmPkW/ymAM2iXnPOYpHfgq60GBQUg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=Lp9tqr8W; arc=none smtp.client-ip=209.85.218.49 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Lp9tqr8W" Received: by mail-ej1-f49.google.com with SMTP id a640c23a62f3a-b8f97c626aaso891168066b.2 for ; Tue, 05 May 2026 00:31:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1777966307; x=1778571107; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=gMh3yQZ+wRAzQ9zOOPwqvnWFjCf8rVBZ0ULEtjFxlWM=; b=Lp9tqr8WsabAdd29mNB8AoIdYZgJTCbwzdhiKrnGWglykp3hwCahaYuz1IORGH/+dB 5ZeXpkZCv+I5j/yYvsaCri+0JKj1NyioM8Yd6mejdPNXGKyjcVXtnH48DXkxJyodxu9X C8NKxMd9bi2h2hnS2/CJ4mEjJGf8ozu1ph2Fslbgg5niipnonRqsfYhAc1a1gKVu0Ol0 +Y1iVf3wf2lwf7Uje4LErQRBVOJur9oK0PLvFojn6AHgA2i6k0Bt2Z0Pr4UGMWkJHn2/ fmisJVbhHoghV49WYGnQwgiQzB1B/uXmeDSaVZ0E4nT7EtNi9p//UymieO9bLvFljGN0 n8Ww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1777966307; x=1778571107; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=gMh3yQZ+wRAzQ9zOOPwqvnWFjCf8rVBZ0ULEtjFxlWM=; b=ft3DxR4hcWs602TBR70ueJuWCZ62xAziLyh6jmMzQ+wOmChlsrLErHR0lqPCWWbLH9 w66pujoPJq1W2AR3E7TN4jZna66O/5Kxv2kYo3792vJzrNnJCzk3lzZbkgF90DVZIKwZ +v3oxZvX4x5gJfGahbstvCv6BTuWBDEQblB9FZTknBjzIK+m6/APiYyxGIB8OEsXsgJ7 8Izc4Edo3h+OJ3cgWKGo4JLe6GONiScX7BD0CyMxcoeqX4zE9YwaOX6ojBj5Jg3oEJad YoRvgXRqVLI3JluIqim9OxBKKjqI20lUTEPiejIeP/TDR3swoXzOnB79owvcwfsX3/sU OKpw== X-Forwarded-Encrypted: i=1; AFNElJ98dym87NMDUmHuUMq1pICDT9g0tziwdux1vCbskVrHjuKlzI9CuHaEPM/4hAs38QvSO3QoycRhahA4O+g=@vger.kernel.org X-Gm-Message-State: AOJu0YwLuEACr18gFURA4FAQkgom4kChbhC20ZLLqyrofAP7Re/HlTIp /uYS72GU8Iqvsz9rLtGgTU9UMXQaRXTr3UfqdLL1bjts/6bBR40LxsuC X-Gm-Gg: AeBDieuNJ3wuEOf0B52I76M3G3raglO+L0lRddW3pdz7qE5nkeq+tQyt0JsWIhyhZEU DHhJsGhaEM5J465Cbwt/b3RnG0IiEgZMcYfofZBU7N278j+v1c05C3xfHJlbUY8GPqMbz38CBrg PZvrU31eTpMjBAzFmjUpbFsBDzGli4xpAn6pJcmWtUJvlXcOIYbjtx1awuAjSU1MKZ7lwyWFJoV mKRZJKV83DWFBzJTFqlmZHIG08eqOadyKSDFYk1/uR/7rGNyfhb6tJ4yrF2kNQoiTW0zp9hR7uA Vws2bhIDC0ce9tTxpNsC53EADTNcbTUxxscf6+lzIHxCKyhyL1aA16MmpMdj0nxO9dJSC9cVUjD eI2vZC7R/4OevKjL8cDN5H4ZwZ9EBfGOqV5XeC55dgYYqehbDLzSZbhy8bb4eO7vEP5bm3eS/HY P5bKNYllOvTQqAJJJBKiKJRck= X-Received: by 2002:a17:906:c104:b0:bc2:d403:bcb4 with SMTP id a640c23a62f3a-bc2d403bd4amr350623566b.19.1777966306972; Tue, 05 May 2026 00:31:46 -0700 (PDT) Received: from pc ([196.235.252.38]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-bbe69f6b7f6sm462628266b.3.2026.05.05.00.31.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 May 2026 00:31:46 -0700 (PDT) Date: Tue, 5 May 2026 08:31:43 +0100 From: Salah Triki To: Crt Mori Cc: Jonathan Cameron , Andy Shevchenko , David Lechner , Nuno =?iso-8859-1?Q?S=E1?= , Andy Shevchenko , linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] iio: mlx90614: fix missing GPIO direction return value checks Message-ID: References: <20260427215800.28082-1-salah.triki@gmail.com> <20260428170412.41791bdb@jic23-huawei> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Tue, Apr 28, 2026 at 06:33:05PM +0200, Crt Mori wrote: > On Tue, 28 Apr 2026 at 18:04, Jonathan Cameron wrote: > > > > On Tue, 28 Apr 2026 09:38:47 +0100 > > Salah Triki wrote: > > > > > On Tue, Apr 28, 2026 at 11:00:04AM +0300, Andy Shevchenko wrote: > > > > On Mon, Apr 27, 2026 at 10:58:00PM +0100, Salah Triki wrote: > > > > > The functions gpiod_direction_output() and gpiod_direction_input() can > > > > > fail, but their return values were previously ignored. > > > > > > > > > > If an error occurs during the GPIO configuration, the function should > > > > > abort the wake-up sequence and return the error code. More importantly, > > > > > failing to check these values could lead to the I2C bus remaining > > > > > locked if an error occurs after i2c_lock_bus() is called. > > > > > > > > > > Add return value checks and ensure the I2C bus is properly unlocked > > > > > via a goto label in case of failure. > > > > > > > > ... > > > > > > > > > - gpiod_direction_output(data->wakeup_gpio, 0); > > > > > + > > > > > + ret = gpiod_direction_output(data->wakeup_gpio, 0); > > > > > + if (ret) > > > > > + goto out_unlock; > > > > > + > > > > > msleep(chip_info->wakeup_delay_ms); > > > > > - gpiod_direction_input(data->wakeup_gpio); > > > > > + > > > > > + ret = gpiod_direction_input(data->wakeup_gpio); > > > > > + if (ret) > > > > > + goto out_unlock; > > > > > > > > While technically it sounds correct, the potential problem here is that you may > > > > fail this in case CONFIG_GPIOLIB=n. Is this GPIO optional? > > > > What may happen if GPIO is not optional, but for some reason setting it fails? > > > > > > > > TL;DR: > > > > I am not sure about this patch. At least I'm not comfortable to take it without > > > > testing on real HW. > > > > > > > > -- > > > > With Best Regards, > > > > Andy Shevchenko > > > > > > > > > > > > > > Thank you for your feedback. Since I don't have the physical hardware to > > > perform the necessary tests and ensure there are no regressions, I agree > > > that it is safer to drop this patch. I don't want to risk breaking the > > > driver for a minor cleanup. > > > > > Maybe Crt has the hardware to test. For now I'll assume this is not > > going anywhere wrt to tracking in patchwork. > > > I will dig the hardware to test as it was off my desk for some time > now. It seems like backwards compatible, but I am wondering why would > we want to be informed of this failing? The problem is that sensor > uses wakeup line (same as for i2c) to receive wakeup pulse, so in > theory this is already a workaround. @Salah did you see any issues in > the wild? > Thanks for digging out the hardware to test this! To answer your question: I haven't observed this failing in the wild on this specific sensor. The patch was initially motivated by a static analysis and general robustness cleanup, ensuring that we don't ignore errors from gpiod_* functions (especially in the context of keeping the I2C bus locked). However, as Andy pointed out, turning this into a fatal error might not be desirable if the GPIO is optional or if the failure is transient on some platforms. The goal was simply to improve visibility if the GPIO configuration fails. Best Regards, -- Salah Triki