From patchwork Mon Dec 20 17:41:53 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cristian Marussi X-Patchwork-Id: 12696866 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 19A33C433EF for ; Mon, 20 Dec 2021 17:43:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:MIME-Version:List-Subscribe:List-Help: List-Post:List-Archive:List-Unsubscribe:List-Id:Message-Id:Date:Subject:Cc:To :From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=jyxAz2BV85pS2Q2PQj+4TI+H7W4QcEwn0FhTyL1Yn1A=; b=T4Urgwwj7XKv5a CAop3UAe0MWPY9bcnNN25lPJpeKf+f8cH0xuwFR1kEzA2krQp+fZoEfJnPBb+d3k2JZnP1NCGi6nA 3BEp6O9uFA/qJnysri4kvny5CHV4afwXcY0mVs0ghdikClAEhBpmbWBHJwOpbfj9kwUHZa8pWwVBn YBjtyyknEP3nvre3m7sWL/jbyUVIab3eqlW2zd4LOWsqMA3EC+4lzglkHCTo9bKcIYcgS5DI4ANv8 xK4Bc7Clsk02KoJJdv7kT89J/vCJvLCpxUG20EXw5sL5qaMoaFwpEqsaTwmZDzs9SaYWSoLtMXd6e EQI3C8L+2CghhuxKPCLA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mzMg3-003cM3-PH; Mon, 20 Dec 2021 17:42:23 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mzMfz-003cJb-Qg for linux-arm-kernel@lists.infradead.org; Mon, 20 Dec 2021 17:42:21 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 4992A6D; Mon, 20 Dec 2021 09:42:16 -0800 (PST) Received: from e120937-lin.home (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E5E4D3F774; Mon, 20 Dec 2021 09:42:14 -0800 (PST) From: Cristian Marussi To: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Cc: sudeep.holla@arm.com, f.fainelli@gmail.com, souvik.chakravarty@arm.com, nicola.mazzucato@arm.com, Cristian Marussi Subject: [RFC PATCH 0/2] Sensor readings fixes Date: Mon, 20 Dec 2021 17:41:53 +0000 Message-Id: <20211220174155.40239-1-cristian.marussi@arm.com> X-Mailer: git-send-email 2.17.1 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211220_094219_953162_3A07FD59 X-CRM114-Status: GOOD ( 12.45 ) 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: , MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi, this was supposed to be an easy fix on how sensor readings are handled across different FW versions while maintaining backward compatibility, but the solution raised for me more questions than the issue itself... ...so I posted as an RFC. In a nutshell, since SCMI FWv3.0 spec, sensors SCMI_READING_GET command can report axis and timestamps too, beside readings, so a brand new scmi_reading_get_timestamped protocol operation was exposed (used by IIO) while the old scmi_reading_get was kept as it was, already used by HWMON subsystem for other classes of sensors. Unfortunately, also the flavour of reported values changed from unsigned to signed with v3.0, so if you end-up on a system running against an SCMI v3.0 FW platform you could end up reading a negative value and interpreting it as a big positive since scmi_reading_get reports only u64. 01/02 simply takes care, when a FW >= 3.0 is detected, to return an error to any scmi_reading_get request if that would result in tryinh to carry a negative value into an u64. So this should rectify the API exposed by SCMI sensor and make it consistent in general, in such a way that a user calling it won't risk to receive a false big-positive which was indeed a 2-complement negative from the perpective of the SCMI fw. So far so good...sort of...since, to make things more dire, the HWMON interface, which is the only current upstream user of scmi_reading_get DOES allow indeed to report to the HWMON core negative values, so it was just that we were silently interpreting u64 as s64 :P ... ...as a consequence the fix above to the SCMI API will potentially break this undocumented behaviour of our only scmi_reading_get user. Additionally, while looking at this, I realized that for similar reasons even on systems running the current SCMI stack API and an old FW <=2.0 the current HWMON read is potentially broken, since when the FW reports a very big and real positive number we'll report it as a signed long to the HWMON core, so turning it wrongly into a negative report: for this reason 02/02 adds a check inside scmi-hwmon to filter out, reporting errors, any result reported by scmi_reading_get so big as to be considered a negative in 2-complement... ...and this will probably break even more the undocumented behaviours... Any feedback welcome ! Thanks, Cristian Cristian Marussi (2): firmware: arm_scmi: Filter out negative results on scmi_reading_get hwmon: (scmi) Filter out results wrongly interpreted as negatives drivers/firmware/arm_scmi/sensors.c | 8 ++++++++ drivers/hwmon/scmi-hwmon.c | 14 ++++++++++++++ 2 files changed, 22 insertions(+)