linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eliav Farber <farbere@amazon.com>
To: <jdelvare@suse.com>, <linux@roeck-us.net>, <robh+dt@kernel.org>,
	<p.zabel@pengutronix.de>, <rtanwar@maxlinear.com>,
	<andriy.shevchenko@intel.com>, <linux-hwmon@vger.kernel.org>,
	<devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Cc: <farbere@amazon.com>, <hhhawa@amazon.com>, <jonnyc@amazon.com>,
	"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>
Subject: [PATCH v5 04/21] hwmon: (mr75203) fix voltage equation for negative source input
Date: Thu, 8 Sep 2022 15:24:32 +0000	[thread overview]
Message-ID: <20220908152449.35457-5-farbere@amazon.com> (raw)
In-Reply-To: <20220908152449.35457-1-farbere@amazon.com>

According to Moortec Embedded Voltage Monitor (MEVM) series 3 data
sheet, the minimum input signal is -100mv and maximum input signal
is +1000mv.

The equation used to convert the digital word to voltage uses mixed
types (*val signed and n unsigned), and on 64 bit machines also has
different size, since sizeof(u32) = 4 and sizeof(long) = 8.

So when measuring a negative input, n will be small enough, such that
PVT_N_CONST * n < PVT_R_CONST, and the result of
(PVT_N_CONST * n - PVT_R_CONST) will overflow to a very big positive
32 bit number. Then when storing the result in *val it will be the same
value just in 64 bit (instead of it representing a negative number which
will what happen when sizeof(long) = 4).

When -1023 <= (PVT_N_CONST * n - PVT_R_CONST) <= -1
dividing the number by 1024 should result of in 0, but because ">> 10"
is used, and the sign bit is used to fill the vacated bit positions, it
results in -1 (0xf...fffff) which is wrong.

This change fixes the sign problem and supports negative values by
casting n to long and replacing the shift right with div operation.

Fixes: 9d823351a337 ("hwmon: Add hardware monitoring driver for Moortec MR75203 PVT controller")
Signed-off-by: Eliav Farber <farbere@amazon.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
V5 -> V4:
- Add detailed comment in code explaining the cast and the div instead of right
  shift or use of BIT().

V4 -> V3:
- Remove unrelated change (add of empty line).

V3 -> V2:
- Fix equation to support negative values instead of limiting value to zero.

 drivers/hwmon/mr75203.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c
index 62df0c9498f3..8211d463495d 100644
--- a/drivers/hwmon/mr75203.c
+++ b/drivers/hwmon/mr75203.c
@@ -201,8 +201,18 @@ static int pvt_read_in(struct device *dev, u32 attr, int channel, long *val)
 			return ret;
 
 		n &= SAMPLE_DATA_MSK;
-		/* Convert the N bitstream count into voltage */
-		*val = (PVT_N_CONST * n - PVT_R_CONST) >> PVT_CONV_BITS;
+		/*
+		 * Convert the N bitstream count into voltage.
+		 * To support negative voltage calculation for 64bit machines
+		 * n must be cast to long, since n and *val differ both in
+		 * signedness and in size.
+		 * Division is used instead of right shift, because for signed
+		 * numbers, the sign bit is used to fill the vacated bit
+		 * positions, and if the number is negative, 1 is used.
+		 * BIT(x) may not be used instead of (1 << x) because it's
+		 * unsigned.
+		 */
+		*val = (PVT_N_CONST * (long)n - PVT_R_CONST) / (1 << PVT_CONV_BITS);
 
 		return 0;
 	default:
-- 
2.37.1


  parent reply	other threads:[~2022-09-08 15:31 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-08 15:24 [PATCH v5 00/21] Variety of fixes and new features for mr75203 driver Eliav Farber
2022-09-08 15:24 ` [PATCH v5 01/21] dt-bindings: hwmon: (mr75203) fix "intel,vm-map" property to be optional Eliav Farber
2022-09-08 18:59   ` Guenter Roeck
2022-09-08 15:24 ` [PATCH v5 02/21] hwmon: (mr75203) fix VM sensor allocation when "intel,vm-map" not defined Eliav Farber
2022-09-08 15:24 ` [PATCH v5 03/21] hwmon: (mr75203) update pvt->v_num and vm_num to the actual number of used sensors Eliav Farber
2022-09-08 15:24 ` Eliav Farber [this message]
2022-09-08 15:24 ` [PATCH v5 05/21] hwmon: (mr75203) fix multi-channel voltage reading Eliav Farber
2022-09-08 15:24 ` [PATCH v5 06/21] hwmon: (mr75203) enable polling for all VM channels Eliav Farber
2022-09-08 15:24 ` [PATCH v5 07/21] dt-bindings: hwmon: (mr75203) add description for Moortec's PVT controller Eliav Farber
2022-09-09  2:16   ` Guenter Roeck
2022-09-08 15:24 ` [PATCH v5 08/21] dt-bindings: hwmon: (mr75203) change "resets" property to be optional Eliav Farber
2022-09-09  2:17   ` Guenter Roeck
2022-09-08 15:24 ` [PATCH v5 09/21] hwmon: (mr75203) skip reset-control deassert for SOCs that don't support it Eliav Farber
2022-09-09  2:18   ` Guenter Roeck
2022-09-08 15:24 ` [PATCH v5 10/21] dt-bindings: vendor-prefixes: add vendor prefix for Moortec Eliav Farber
2022-09-13 11:47   ` Rob Herring
2022-09-19 12:59   ` Guenter Roeck
2022-09-08 15:24 ` [PATCH v5 11/21] dt-bindings: hwmon: (mr75203) add "moortec,vm-active-channels" property Eliav Farber
2022-09-13 11:49   ` Rob Herring
2022-09-19 12:59   ` Guenter Roeck
2022-09-08 15:24 ` [PATCH v5 12/21] hwmon: (mr75203) add VM active channel support Eliav Farber
2022-09-19 13:00   ` Guenter Roeck
2022-09-08 15:24 ` [PATCH v5 13/21] dt-bindings: hwmon: (mr75203) add "moortec,vm-pre-scaler-x2" property Eliav Farber
2022-09-13 11:50   ` Rob Herring
2022-09-19 13:01   ` Guenter Roeck
2022-09-08 15:24 ` [PATCH v5 14/21] hwmon: (mr75203) add VM pre-scaler x2 support Eliav Farber
2022-09-19 13:10   ` Guenter Roeck
2022-09-08 15:24 ` [PATCH v5 15/21] hwmon: (mr75203) modify the temperature equation according to series 5 datasheet Eliav Farber
2022-09-19 13:11   ` Guenter Roeck
2022-09-08 15:24 ` [PATCH v5 16/21] dt-bindings: hwmon: (mr75203) add "moortec,ts-series" property Eliav Farber
2022-09-08 19:02   ` Guenter Roeck
2022-09-08 20:18     ` [PATCH v5 16/21] dt-bindings: hwmon: (mr75203) add "moortec, ts-series" property Farber, Eliav
2022-09-08 20:22       ` Guenter Roeck
2022-09-13 11:57   ` [PATCH v5 16/21] dt-bindings: hwmon: (mr75203) add "moortec,ts-series" property Rob Herring
2022-09-19 13:18   ` Guenter Roeck
2022-09-08 15:24 ` [PATCH v5 17/21] hwmon: (mr75203) add support for series 6 temperature equation Eliav Farber
2022-09-19 13:19   ` Guenter Roeck
2022-09-08 15:24 ` [PATCH v5 18/21] dt-bindings: hwmon: (mr75203) add coefficient properties for the thermal equation Eliav Farber
2022-09-13 11:58   ` Rob Herring
2022-09-19 13:20   ` Guenter Roeck
2022-09-08 15:24 ` [PATCH v5 19/21] hwmon: (mr75203) parse temperature coefficients from device-tree Eliav Farber
2022-09-19 13:21   ` Guenter Roeck
2022-09-08 15:24 ` [PATCH v5 20/21] hwmon: (mr75203) add debugfs to read and write temperature coefficients Eliav Farber
2022-09-08 18:11   ` Andy Shevchenko
2022-09-13 13:06     ` Farber, Eliav
2022-09-13 14:40       ` Farber, Eliav
2022-09-13 17:01         ` Andy Shevchenko
2022-09-14  4:26           ` Farber, Eliav
2022-09-14  9:32             ` Andy Shevchenko
2022-09-14 14:03               ` Andy Shevchenko
2022-09-14 14:08                 ` Andy Shevchenko
2022-09-15  9:17                   ` Farber, Eliav
2022-09-19 13:22   ` Guenter Roeck
2022-09-08 15:24 ` [PATCH v5 21/21] hwmon: (mr75203) fix coding style space errors Eliav Farber
2022-09-19 13:23   ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220908152449.35457-5-farbere@amazon.com \
    --to=farbere@amazon.com \
    --cc=andriy.shevchenko@intel.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=devicetree@vger.kernel.org \
    --cc=hhhawa@amazon.com \
    --cc=jdelvare@suse.com \
    --cc=jonnyc@amazon.com \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=p.zabel@pengutronix.de \
    --cc=robh+dt@kernel.org \
    --cc=rtanwar@maxlinear.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).