From patchwork Wed Dec 7 18:44:08 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sudeep Holla X-Patchwork-Id: 87160 Delivered-To: patch@linaro.org Received: by 10.140.20.101 with SMTP id 92csp467739qgi; Wed, 7 Dec 2016 10:46:04 -0800 (PST) X-Received: by 10.99.60.11 with SMTP id j11mr124801461pga.26.1481136364644; Wed, 07 Dec 2016 10:46:04 -0800 (PST) Return-Path: Received: from bombadil.infradead.org (bombadil.infradead.org. [2001:1868:205::9]) by mx.google.com with ESMTPS id o6si25093571plh.284.2016.12.07.10.46.04 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 07 Dec 2016 10:46:04 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 2001:1868:205::9 as permitted sender) client-ip=2001:1868:205::9; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 2001:1868:205::9 as permitted sender) smtp.mailfrom=linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1cEhCq-0006MW-FW; Wed, 07 Dec 2016 18:44:40 +0000 Received: from foss.arm.com ([217.140.101.70]) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1cEhCh-0006Dz-0u; Wed, 07 Dec 2016 18:44:35 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 642F5AD7; Wed, 7 Dec 2016 10:44:10 -0800 (PST) Received: from [10.1.210.28] (e107155-lin.cambridge.arm.com [10.1.210.28]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 8583B3F445; Wed, 7 Dec 2016 10:44:09 -0800 (PST) Subject: Re: [PATCH] firmware: arm_scpi: fix reading sensor values on pre-1.0 SCPI firmwares To: Martin Blumenstingl , linux-arm-kernel@lists.infradead.org, linux-amlogic@lists.infradead.org References: <20161124001845.20830-1-martin.blumenstingl@googlemail.com> <20161124001845.20830-2-martin.blumenstingl@googlemail.com> From: Sudeep Holla Organization: ARM Message-ID: <663deed4-fbbb-20ad-1943-dbd4e66e797f@arm.com> Date: Wed, 7 Dec 2016 18:44:08 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <20161124001845.20830-2-martin.blumenstingl@googlemail.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20161207_104431_119086_4E2AD5C5 X-CRM114-Status: GOOD ( 13.75 ) X-Spam-Score: -8.3 (--------) X-Spam-Report: SpamAssassin version 3.4.0 on bombadil.infradead.org summary: Content analysis details: (-8.3 points) pts rule name description ---- ---------------------- -------------------------------------------------- -5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/, high trust [217.140.101.70 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -1.4 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: narmstrong@baylibre.com, Sudeep Holla Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org On 24/11/16 00:18, Martin Blumenstingl wrote: > The pre-1.0 SCPI firmwares are using one __le32 as sensor value, while > the 1.0 SCPI protocol uses two __le32 as sensor values (a total of > 64bit, split into 32bit upper and 32bit lower value). > Using an "struct sensor_value" to read the sensor value on a pre-1.0 > SCPI firmware gives garbage in the "hi_val" field. Introducing a > separate function which handles scpi_ops.sensor_get_value for pre-1.0 > SCPI firmware implementations ensures that we do not read memory which > was not written by the SCPI firmware (which fixes for example the > temperature reported by scpi-hwmon). > > Signed-off-by: Martin Blumenstingl > --- > drivers/firmware/arm_scpi.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c > index 70e1323..19f750d 100644 > --- a/drivers/firmware/arm_scpi.c > +++ b/drivers/firmware/arm_scpi.c > @@ -728,6 +728,20 @@ static int scpi_sensor_get_value(u16 sensor, u64 *val) > return ret; > } > > +static int legacy_scpi_sensor_get_value(u16 sensor, u64 *val) > +{ > + __le16 id = cpu_to_le16(sensor); > + __le32 value; > + int ret; > + > + ret = scpi_send_message(CMD_SENSOR_VALUE, &id, sizeof(id), > + &value, sizeof(value)); > + if (!ret) > + *val = le32_to_cpu(value); > + > + return ret; > +} Instead of duplicating the code so much, can we just manage something like this. If more commands need Rx len, then we can think of adding that. Even then reseting shared buffers is not good, we can clear the buffers on the stack. static int scpi_device_get_power_state(u16 dev_id) -- Regards, Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel diff --git i/drivers/firmware/arm_scpi.c w/drivers/firmware/arm_scpi.c index 70e13230d8db..04aa873205e9 100644 --- i/drivers/firmware/arm_scpi.c +++ w/drivers/firmware/arm_scpi.c @@ -721,11 +721,15 @@ static int scpi_sensor_get_value(u16 sensor, u64 *val) ret = scpi_send_message(CMD_SENSOR_VALUE, &id, sizeof(id), &buf, sizeof(buf)); - if (!ret) + if (ret) + return ret; + + if (scpi_info->is_legacy) /* 32-bits supported, hi_val can be junk */ + *val = le32_to_cpu(buf.lo_val); + else *val = (u64)le32_to_cpu(buf.hi_val) << 32 | le32_to_cpu(buf.lo_val); - - return ret; + return 0; }