All of lore.kernel.org
 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:30 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.