All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicolin Chen <nicoleotsuka@gmail.com>
To: "Brüns, Stefan" <Stefan.Bruens@rwth-aachen.de>
Cc: "jdelvare@suse.com" <jdelvare@suse.com>,
	"linux@roeck-us.net" <linux@roeck-us.net>,
	"linux-hwmon@vger.kernel.org" <linux-hwmon@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"corbet@lwn.net" <corbet@lwn.net>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>
Subject: Re: [RFC][PATCH] hwmon: (ina2xx) Improve current and power reading precision
Date: Fri, 4 Jan 2019 17:26:43 -0800	[thread overview]
Message-ID: <20190105012642.GA18025@Asurada-Nvidia.nvidia.com> (raw)
In-Reply-To: <1717545.GXSegKtrMu@sbruens-linux.lcs.intern>

Hi Stefan,

Sorry for a super late reply. I took a long vacation.

On Wed, Nov 21, 2018 at 10:16:09PM +0000, Brüns, Stefan wrote:
> > > Another concern may be voltage drop over the shunt, but for this case you
> > > have a nominal voltage of 1.8V, so 30uV are 0.001%.
> > > 
> > > > When measuring a 1.8v voltage running a small current (e.g. 33 mA),
> > > > the power value (that's supposed to be 59.4 mW) becomes inaccurate
> > > > due to the larger scale (25mA for method A; 62.5 mA for method B).
> > 
> > Just found out that I have typos here: 25mW and 62.5mW.
> > 
> > > Another look into the datasheet reveals, even at full gain (PGA=1), the
> > > LSB is 40mV / 2^12 = 40mV / 4096 ~ 10uV. So when the current ADC reads
> > > out as 3*LSB, this anything between 25mA and 35mA. This is the best case
> > > figure.
> > Current read doesn't get affected a lot actually, since hwmon ABI
> > also reports current value in unit mA. However, the power read is
> > the matter here. With a 62.5mW power_lsb, power results are kinda
> > useless on my system.
> 
> The reported current does not matter here, actually. Internally, the ADC value 
> will have an uncertainty of 10mA (at PGA=1). At 1.8V, your uncertainty is 
> 18mW. And thats *only* the quantization noise. It wont get better than that.

The fact is that I do get better power results after setting the
calibration value to 0x7ff. That's the necessity of this change.

> Also note, you are apparently using the ina2xx hwmon driver - I strongly 
> advise against it, you should either use the ina2xx driver from the IIO 
> subsystem directly, or use the IIO driver via iio-hwmon.

The IIO version is also using the minimum calibration value. It
will not solve my problem here.

> There is also always the possibility to read the bus and shunt voltage 
> registers and calculate the power manually.

Won't that be a waste since the hardware could have provided a
better accuracy? It would need more I2C bus reads and cpu cycles
for calculation.

I don't get why you're against a setting for calibration value.
This is how the hardware got designed to cover different cases.
Since we do have such a case that needs some accuracy, it'd be
fair to add it into the driver. Plus, the feature won't change
the minimum calibration value at all -- everyone would be happy.

Thanks
Nicolin

WARNING: multiple messages have this Message-ID (diff)
From: Nicolin Chen <nicoleotsuka@gmail.com>
To: linux-hwmon@vger.kernel.org
Subject: Re: [RFC][PATCH] hwmon: (ina2xx) Improve current and power reading precision
Date: Fri, 04 Jan 2019 17:26:58 -0800	[thread overview]
Message-ID: <20190105012642.GA18025@Asurada-Nvidia.nvidia.com> (raw)
In-Reply-To: <20181121012629.5432-1-nicoleotsuka@gmail.com>

Hi Stefan,

Sorry for a super late reply. I took a long vacation.

On Wed, Nov 21, 2018 at 10:16:09PM +0000, Brüns, Stefan wrote:
> > > Another concern may be voltage drop over the shunt, but for this case you
> > > have a nominal voltage of 1.8V, so 30uV are 0.001%.
> > > 
> > > > When measuring a 1.8v voltage running a small current (e.g. 33 mA),
> > > > the power value (that's supposed to be 59.4 mW) becomes inaccurate
> > > > due to the larger scale (25mA for method A; 62.5 mA for method B).
> > 
> > Just found out that I have typos here: 25mW and 62.5mW.
> > 
> > > Another look into the datasheet reveals, even at full gain (PGA=1), the
> > > LSB is 40mV / 2^12 = 40mV / 4096 ~ 10uV. So when the current ADC reads
> > > out as 3*LSB, this anything between 25mA and 35mA. This is the best case
> > > figure.
> > Current read doesn't get affected a lot actually, since hwmon ABI
> > also reports current value in unit mA. However, the power read is
> > the matter here. With a 62.5mW power_lsb, power results are kinda
> > useless on my system.
> 
> The reported current does not matter here, actually. Internally, the ADC 
> value 
> will have an uncertainty of 10mA (at PGA=1). At 1.8V, your uncertainty is 
> 18mW. And thats *only* the quantization noise. It wont get better than that.

The fact is that I do get better power results after setting the
calibration value to 0x7ff. That's the necessity of this change.

> Also note, you are apparently using the ina2xx hwmon driver - I strongly 
> advise against it, you should either use the ina2xx driver from the IIO 
> subsystem directly, or use the IIO driver via iio-hwmon.

The IIO version is also using the minimum calibration value. It
will not solve my problem here.

> There is also always the possibility to read the bus and shunt voltage 
> registers and calculate the power manually.

Won't that be a waste since the hardware could have provided a
better accuracy? It would need more I2C bus reads and cpu cycles
for calculation.

I don't get why you're against a setting for calibration value.
This is how the hardware got designed to cover different cases.
Since we do have such a case that needs some accuracy, it'd be
fair to add it into the driver. Plus, the feature won't change
the minimum calibration value at all -- everyone would be happy.

Thanks
Nicolin

  parent reply	other threads:[~2019-01-05  1:26 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-21  1:26 [RFC][PATCH] hwmon: (ina2xx) Improve current and power reading precision Nicolin Chen
2018-11-21 16:13 ` Brüns, Stefan
2018-11-21 19:40   ` Nicolin Chen
2018-11-21 19:40     ` Nicolin Chen
2018-11-21 22:16     ` Brüns, Stefan
2018-11-21 22:57       ` Guenter Roeck
2019-01-05  1:26       ` Nicolin Chen [this message]
2019-01-05  1:26         ` Nicolin Chen
2019-01-17 22:38         ` Nicolin Chen
2019-01-17 23:13           ` Guenter Roeck
2019-01-17 23:16             ` Nicolin Chen

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=20190105012642.GA18025@Asurada-Nvidia.nvidia.com \
    --to=nicoleotsuka@gmail.com \
    --cc=Stefan.Bruens@rwth-aachen.de \
    --cc=corbet@lwn.net \
    --cc=jdelvare@suse.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    /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.