From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752727AbbJTMyQ (ORCPT ); Tue, 20 Oct 2015 08:54:16 -0400 Received: from bh-25.webhostbox.net ([208.91.199.152]:47410 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750910AbbJTMyO (ORCPT ); Tue, 20 Oct 2015 08:54:14 -0400 Subject: Re: [PATCH v2] hwmon: ina2xx: allow for actual measurement bandwidth above 160 Hz To: Marc Titinger , jdelvare@suse.com References: <56259997.6080706@roeck-us.net> <1445329244-21627-1-git-send-email-mtitinger@baylibre.com> Cc: lm-sensors@lm-sensors.org, linux-kernel@vger.kernel.org, bcousson@baylibre.com, mturquette@baylibre.com From: Guenter Roeck Message-ID: <56263973.300@roeck-us.net> Date: Tue, 20 Oct 2015 05:54:11 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <1445329244-21627-1-git-send-email-mtitinger@baylibre.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Authenticated_sender: linux@roeck-us.net X-OutGoing-Spam-Status: No, score=-1.0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: authenticated_id: linux@roeck-us.net X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/20/2015 01:20 AM, Marc Titinger wrote: > With the current implementation, the driver will prevent a readout at a > pace faster than the default conversion time (2ms) times the averaging > setting, min AVG being 1:1. > > Any sysfs "show" read access from the client app faster than 500 Hz will be > 'cached' by the driver, but actually since do_update reads all 8 registers, > the best achievable measurement rate is roughly 8*800 us (for the time > spent in i2c-core) i.e. <= 156Hz with Beagle Bone Black. > > This change set uses a register mask to allow for the readout of a single > i2c register at a time. Furthermore, performing subsequent reads on the > same register will make use of the ability of the i2c chip to retain the > last reg offset, hence use a shorter i2c message (roughly 400us instead of > 800us spent in i2c-core.c). > That doesn't work. There could be accesses from other sources (such as through i2c-dev, or in multi-master systems) between two reads. > The best readout rate for a single measurement is now around 2kHz. And for > four measurements around (1/(4*800us) = 312 Hz. Since for any readout rate > faster than 160 Hz the interval is set by the i2c transactions completion, > the 'last-update' anti-flooding code will not have a limiting effect in > practice. Hence I also remove the elapsed time checking in the hwmon driver > for ina2xx. > > To summarize, the patch provides a max bandwidth improvement with hwmon > client apps from ~160 Hz to ~320 Hz, and better in single-channel > polling mode. > Overall your patch pretty much re-implements regmap. Since you drop caching, it is also unnecessary to read all registers at a time, so you can just use a function to read _one_ register and returns its value (with retries). Or use regmap. Either case, do_update() and ina2xx_update_device() are no longer needed. If you want to convert the driver to regmap, just look for 'regmap' in drivers/hwmon for examples. Guenter From mboxrd@z Thu Jan 1 00:00:00 1970 From: Guenter Roeck Date: Tue, 20 Oct 2015 12:54:11 +0000 Subject: Re: [lm-sensors] [PATCH v2] hwmon: ina2xx: allow for actual measurement bandwidth above 160 Hz Message-Id: <56263973.300@roeck-us.net> List-Id: References: <56259997.6080706@roeck-us.net> <1445329244-21627-1-git-send-email-mtitinger@baylibre.com> In-Reply-To: <1445329244-21627-1-git-send-email-mtitinger@baylibre.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Marc Titinger , jdelvare@suse.com Cc: lm-sensors@lm-sensors.org, linux-kernel@vger.kernel.org, bcousson@baylibre.com, mturquette@baylibre.com On 10/20/2015 01:20 AM, Marc Titinger wrote: > With the current implementation, the driver will prevent a readout at a > pace faster than the default conversion time (2ms) times the averaging > setting, min AVG being 1:1. > > Any sysfs "show" read access from the client app faster than 500 Hz will be > 'cached' by the driver, but actually since do_update reads all 8 registers, > the best achievable measurement rate is roughly 8*800 us (for the time > spent in i2c-core) i.e. <= 156Hz with Beagle Bone Black. > > This change set uses a register mask to allow for the readout of a single > i2c register at a time. Furthermore, performing subsequent reads on the > same register will make use of the ability of the i2c chip to retain the > last reg offset, hence use a shorter i2c message (roughly 400us instead of > 800us spent in i2c-core.c). > That doesn't work. There could be accesses from other sources (such as through i2c-dev, or in multi-master systems) between two reads. > The best readout rate for a single measurement is now around 2kHz. And for > four measurements around (1/(4*800us) = 312 Hz. Since for any readout rate > faster than 160 Hz the interval is set by the i2c transactions completion, > the 'last-update' anti-flooding code will not have a limiting effect in > practice. Hence I also remove the elapsed time checking in the hwmon driver > for ina2xx. > > To summarize, the patch provides a max bandwidth improvement with hwmon > client apps from ~160 Hz to ~320 Hz, and better in single-channel > polling mode. > Overall your patch pretty much re-implements regmap. Since you drop caching, it is also unnecessary to read all registers at a time, so you can just use a function to read _one_ register and returns its value (with retries). Or use regmap. Either case, do_update() and ina2xx_update_device() are no longer needed. If you want to convert the driver to regmap, just look for 'regmap' in drivers/hwmon for examples. Guenter _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors