From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-dy1-f170.google.com (mail-dy1-f170.google.com [74.125.82.170]) (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 19B0319F121 for ; Sun, 8 Mar 2026 01:22:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.125.82.170 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772932922; cv=none; b=odkEuRvnBGn6wNjI5fMJIcSYlRBM/FRTYAXXK2jZTQjz6HCfLuyGknQDchXOnFNKs5pAKxn0H8s37A8hUoNlHbu6oEY9KCLSNHxmOsBSFPjryTkhQ5Ab4c2zeAoF361L7jJSEppFSCvdZtdfWVSwBkmZs16xyn4iilr0hgSTIUE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772932922; c=relaxed/simple; bh=etOhkSfYLR5ggsnislmjuT7BqCj6aE2dxd3IvUNbs8U=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=gN0JlTvwN5n9/EIHWlxINpNoN1oeQt6XIz1SndHpfNbgHbU93lN985wj7yw7lwXa7pP9C3KL76Q1FSNWO6/LQkf1rzpoJGDC99VLyQfeotUDDL1EDYeVVpoO/wumDs/yKK0Xc4rGbfrMk/W1Sakqr38o9UHJTvk/Er7XqqSojA4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=roeck-us.net; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=HTGSZb0G; arc=none smtp.client-ip=74.125.82.170 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=roeck-us.net 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="HTGSZb0G" Received: by mail-dy1-f170.google.com with SMTP id 5a478bee46e88-2be19f05d7dso3372941eec.1 for ; Sat, 07 Mar 2026 17:22:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1772932920; x=1773537720; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:sender:from:to:cc:subject:date:message-id :reply-to; bh=U+2uW3pVX4s3uqBjVfFNFLLPtvFDpbm5f7Ej5kaVeSc=; b=HTGSZb0G+cdTvzSnE1qKDEWP37OoAmm1ZlHlMIob0J1/bG7DQTC/mItCUICboMB79T u37HOH0Uyv6mywpeNKjFCy5Udfp6fxtTAbydGioIAok9VqHNqgHj5W+a2oVkJOwulCRc D/lwXlnYwo55WdI7xTD4uWkoQH5Z0wEhv0LmW38CNB4lIg6USCXYQmUGOMvpwKjkGCpx Vm+0eCrK69vlROg/7t7kr21/biqVHOZ28DRsnrwYz71KUq5PhnalrPfL4Z1KQjrQNzOE M/6fbOELSWLRYaqKRoUb8dUjBOTlSSanYuBHIZoplw48e2vt93LaKkuWMsII3BNhu7g1 SPuQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1772932920; x=1773537720; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:sender:x-gm-gg:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=U+2uW3pVX4s3uqBjVfFNFLLPtvFDpbm5f7Ej5kaVeSc=; b=Voe02bP5mc6y9fcc2JHp1esQTcaeHviOAVgV3zCUAkY8Cghh/X8WgIDXZg/ctrxyP/ PBuB6hSriZuUPZ7VpD2MS04xXKq/y3vNxusBy2PahLZynTR7i9nfdLBgWuB12CSkqVJ4 vntK+Apad202IMPe3LBVqxEKJLmKgA07AabqTOSQlvyLMiiL3jHJtMaVbUG3DxBh6Cmi OUwHa4rb6M32vdGRJnoV/jPKNtxrLoZaJCOY4n4sID2xZf7iM4gyaWJLizZsG68JJOyP 3PX232s+JFZwk3bo0BLuAtZWIs0R2AoXUuDRm+uCUaFdyXwt3Ssjlil0aYbaa1Zuli98 hicw== X-Forwarded-Encrypted: i=1; AJvYcCXDeRvxj/w1dJ+pJHvws/ZzeVWjU+iqSZ8hGAU1BXMJ+7GHJee0ASUGkvaeLgeDcFMUo0otj0rakrdCkts=@vger.kernel.org X-Gm-Message-State: AOJu0YyeAU+Q9LIWFeKe8YRFh2bjjySwtoEGZyY3lEzfNbHTj4wXXQbf zHP9hwQSOll2hgpCNp+IcN6X4h7KPsrb1oInjj08uMp4vgAOw4aAI/jx X-Gm-Gg: ATEYQzw0NZ9wtzQuX9qNCPEHClCD5tP5KaKKZdBSSNytsUTbjziwO5N2otYg6TUq4nQ CVsOiCwFaunt6D8Z2jaFvUNjcAfaItUd6CusHQkI8s6Te60MdsHnceTISxnVvZ7fOKQsKiByccW J7mHRJv/Euk0KScZi466cT3KqNX4h70UHhpg6a2ias/FC8z2gWaQEtFfq7u9q+9Pey70NrtmDbz LkADFyLgsPBVCbdEPbZpiBlesKRcGckYpJvK5W5lRygV1Tw5aUO+eVoWqgoNP0qy32QQ4N6rbIn aGLgrZdj7drF4Op6KVQ13wvTJ2AO0sEm1BeVxzz7iqXEtOHffxe/x3s/qFv+a0CiINs7HcIjPyO m54P5vePsQM4+0EIEMgiTfmTYUBHvkFU0+HzSBEarAz7qKf2RqlqGU/vIdnowP9zGHGOCCjwpy1 1vyPAV9J62q3UX2IJvH+Dkgm8+33ZFd/v6QAo5 X-Received: by 2002:a05:7300:dc91:b0:2be:2cfe:68b7 with SMTP id 5a478bee46e88-2be4de9a2dfmr2510733eec.11.1772932920083; Sat, 07 Mar 2026 17:22:00 -0800 (PST) Received: from server.roeck-us.net ([2600:1700:e321:62f0:da43:aeff:fecc:bfd5]) by smtp.gmail.com with ESMTPSA id 5a478bee46e88-2be4f8078c6sm5538581eec.5.2026.03.07.17.21.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 07 Mar 2026 17:21:59 -0800 (PST) Sender: Guenter Roeck Date: Sat, 7 Mar 2026 17:21:58 -0800 From: Guenter Roeck To: Tabrez Ahmed Cc: linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org, Shuah Khan , Brigham Campbell Subject: Re: [PATCH v2] hwmon: (ads7871) Fix incorrect error code in voltage_show Message-ID: References: <20260307115226.25757-1-tabreztalks@gmail.com> 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: <20260307115226.25757-1-tabreztalks@gmail.com> On Sat, Mar 07, 2026 at 05:22:26PM +0530, Tabrez Ahmed wrote: > The voltage_show() function returns -1 when the A/D conversion > fails to complete within the polling loop. -1 maps to -EPERM > (operation not permitted), which does not describe the actual > failure. > > Replace this -1 error code with -ETIMEDOUT to better indicate > the timeout condition to userspace. > > Drop the else block after return. > > Note: not runtime tested due to lack of hardware. > > Signed-off-by: Tabrez Ahmed > --- > Changes in v2: > - Dropped unnecessary 'else' block after return as suggested by Guenter Roeck. > > Note: This patch applies on top of my previously submitted patch: > "hwmon: (ads7871) Replace sprintf() with sysfs_emit()" > > drivers/hwmon/ads7871.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwmon/ads7871.c b/drivers/hwmon/ads7871.c > index b84426c940c5e..753bf77ce19b4 100644 > --- a/drivers/hwmon/ads7871.c > +++ b/drivers/hwmon/ads7871.c > @@ -125,9 +125,9 @@ static ssize_t voltage_show(struct device *dev, struct device_attribute *da, > /*result in volts*10000 = (val/8192)*2.5*10000*/ > val = ((val >> 2) * 25000) / 8192; > return sysfs_emit(buf, "%d\n", val); > - } else { > - return -1; > } > + > + return -ETIMEDOUT; AI feedback below. Somewhat unrelated, but it turns out the driver has a number of problems. I'll apply the two patches, but that won't fix the real problems with the driver. Does this code mask actual SPI errors? If ads7871_read_reg8() fails, it returns a negative error code (e.g., -EIO). Since MUX_CNV_BM is 0x80, any standard negative error code will have the 7th bit set, meaning mux_cnv evaluates to 1: ret = ads7871_read_reg8(spi, REG_GAIN_MUX); mux_cnv = ((ret & MUX_CNV_BM) >> MUX_CNV_BV); This causes the polling loop to continue and eventually return -ETIMEDOUT here. Should we check if ret < 0 and propagate the actual SPI error code instead of unconditionally returning -ETIMEDOUT? Also, since this attribute is registered via devm_hwmon_device_register_with_groups() rather than the _with_info() variant, the hwmon core does not serialize accesses. If userspace reads multiple channels concurrently, could they overwrite each other's REG_GAIN_MUX writes before the conversion completes? Should we add locking to serialize this function? Additionally, since we are improving error reporting, should we also check the return values of ads7871_write_reg8() and ads7871_read_reg16() to prevent operating on or returning bogus data when SPI transfers fail? That driver would really benefit from a conversion to the with_info API. Thanks, Guenter