All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@cam.ac.uk>
To: guenter.roeck@ericsson.com
Cc: Jean Delvare <khali@linux-fr.org>,
	Jonathan Cameron <kernel@jic23.retrosnub.co.uk>,
	Randy Dunlap <rdunlap@xenotime.net>,
	Greg Schnorr <gschnorr@cisco.com>,
	"lm-sensors@lm-sensors.org" <lm-sensors@lm-sensors.org>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [lm-sensors] [PATCH v4 4/5] hwmon: (pmbus) Add support for
Date: Sat, 26 Feb 2011 10:43:49 +0000	[thread overview]
Message-ID: <4D68D965.7040307@cam.ac.uk> (raw)
In-Reply-To: <1298669577.1735.99.camel@groeck-laptop>

On 02/25/11 21:32, Guenter Roeck wrote:
> Hi Jonathan,
> 
> On Fri, 2011-02-25 at 15:42 -0500, Jonathan Cameron wrote:
>> One trivial formatting nitpick.
>> Dug out datasheet for this one..
>> I'm a little unclear how value from there map to those stored in here...
> [ ... ]
> 
>>> +static struct pmbus_driver_info max34440_info[] = {
>>> +	[max34440] = {
>>> +		.pages = 14,
>>> +		.direct[PSC_VOLTAGE_IN] = true,
>>> +		.direct[PSC_VOLTAGE_OUT] = true,
>>> +		.direct[PSC_TEMPERATURE] = true,
>>> +		.direct[PSC_CURRENT_OUT] = true,
>>> +		.m[PSC_VOLTAGE_IN] = 1,
>>> +		.b[PSC_VOLTAGE_IN] = 0,
>>> +		.R[PSC_VOLTAGE_IN] = 3,
>>> +		.m[PSC_VOLTAGE_OUT] = 1,
>>> +		.b[PSC_VOLTAGE_OUT] = 0,
>>> +		.R[PSC_VOLTAGE_OUT] = 3,
>>> +		.m[PSC_CURRENT_OUT] = 1,
>>> +		.b[PSC_CURRENT_OUT] = 0,
>>> +		.R[PSC_CURRENT_OUT] = 3,
>> Table 3 of datasheet says R for current is 0... I may be missing
>> something though!  
> 
> At least this one has a relatively simple reason ;).
> 
> The base unit (to which R is applied to) in the datasheet is mV and mA.
> The base unit used by max8688 and max16064 is V and A. Chips in linear
> mode also use V and A. I had the choice of either adjusting the code to
> expect mV and mA for all chips, or to expect V and A for all chips. I
> decided to use the latter for consistency, and to adjust R in the core
> driver to the units expected to be reported in the sysfs attributes. For
> max34440/max34441, the adjustment ends up being 0 either way (3 + (-3)
> in one direction and (-3) + 3 in the other).
This reason actually occurred to me whilst cycling home but thanks for
clarifying this.
> 
> Another option would be to not do any sysfs related adjustments in the
> core driver, but to do all adjustments via platform data instead. I
> decided against that because the code to handle linear mode (which
> expects V and A) would no longer match the code to handle direct mode,
> which might cause even more confusion.
What you have works fine for me though you'll have to keep a close eye
on any drivers submitted by others as this will be easy to get wrong
as people will blindly fill tables in from datasheets.

Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
> 
> Thanks,
> Guenter
> 
> 
> 


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

WARNING: multiple messages have this Message-ID (diff)
From: Jonathan Cameron <jic23@cam.ac.uk>
To: guenter.roeck@ericsson.com
Cc: Jean Delvare <khali@linux-fr.org>,
	Jonathan Cameron <kernel@jic23.retrosnub.co.uk>,
	Randy Dunlap <rdunlap@xenotime.net>,
	Greg Schnorr <gschnorr@cisco.com>,
	"lm-sensors@lm-sensors.org" <lm-sensors@lm-sensors.org>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 4/5] hwmon: (pmbus) Add support for Maxim MAX34440/MAX34441
Date: Sat, 26 Feb 2011 10:43:49 +0000	[thread overview]
Message-ID: <4D68D965.7040307@cam.ac.uk> (raw)
In-Reply-To: <1298669577.1735.99.camel@groeck-laptop>

On 02/25/11 21:32, Guenter Roeck wrote:
> Hi Jonathan,
> 
> On Fri, 2011-02-25 at 15:42 -0500, Jonathan Cameron wrote:
>> One trivial formatting nitpick.
>> Dug out datasheet for this one..
>> I'm a little unclear how value from there map to those stored in here...
> [ ... ]
> 
>>> +static struct pmbus_driver_info max34440_info[] = {
>>> +	[max34440] = {
>>> +		.pages = 14,
>>> +		.direct[PSC_VOLTAGE_IN] = true,
>>> +		.direct[PSC_VOLTAGE_OUT] = true,
>>> +		.direct[PSC_TEMPERATURE] = true,
>>> +		.direct[PSC_CURRENT_OUT] = true,
>>> +		.m[PSC_VOLTAGE_IN] = 1,
>>> +		.b[PSC_VOLTAGE_IN] = 0,
>>> +		.R[PSC_VOLTAGE_IN] = 3,
>>> +		.m[PSC_VOLTAGE_OUT] = 1,
>>> +		.b[PSC_VOLTAGE_OUT] = 0,
>>> +		.R[PSC_VOLTAGE_OUT] = 3,
>>> +		.m[PSC_CURRENT_OUT] = 1,
>>> +		.b[PSC_CURRENT_OUT] = 0,
>>> +		.R[PSC_CURRENT_OUT] = 3,
>> Table 3 of datasheet says R for current is 0... I may be missing
>> something though!  
> 
> At least this one has a relatively simple reason ;).
> 
> The base unit (to which R is applied to) in the datasheet is mV and mA.
> The base unit used by max8688 and max16064 is V and A. Chips in linear
> mode also use V and A. I had the choice of either adjusting the code to
> expect mV and mA for all chips, or to expect V and A for all chips. I
> decided to use the latter for consistency, and to adjust R in the core
> driver to the units expected to be reported in the sysfs attributes. For
> max34440/max34441, the adjustment ends up being 0 either way (3 + (-3)
> in one direction and (-3) + 3 in the other).
This reason actually occurred to me whilst cycling home but thanks for
clarifying this.
> 
> Another option would be to not do any sysfs related adjustments in the
> core driver, but to do all adjustments via platform data instead. I
> decided against that because the code to handle linear mode (which
> expects V and A) would no longer match the code to handle direct mode,
> which might cause even more confusion.
What you have works fine for me though you'll have to keep a close eye
on any drivers submitted by others as this will be easy to get wrong
as people will blindly fill tables in from datasheets.

Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
> 
> Thanks,
> Guenter
> 
> 
> 


  reply	other threads:[~2011-02-26 10:43 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-17 19:00 [lm-sensors] [PATCH v4 0/5] hwmon: PMBus device driver Guenter Roeck
2011-02-17 19:00 ` Guenter Roeck
2011-02-17 19:00 ` [lm-sensors] [PATCH v4 1/5] " Guenter Roeck
2011-02-17 19:00   ` Guenter Roeck
2011-02-25 20:23   ` [lm-sensors] " Jonathan Cameron
2011-02-25 20:23     ` Jonathan Cameron
2011-02-26  2:45     ` [lm-sensors] " Guenter Roeck
2011-02-26  2:45       ` Guenter Roeck
2011-02-26 10:41       ` [lm-sensors] " Jonathan Cameron
2011-02-26 10:41         ` Jonathan Cameron
2011-02-26 15:19         ` [lm-sensors] " Guenter Roeck
2011-02-26 15:19           ` Guenter Roeck
2011-02-17 19:00 ` [lm-sensors] [PATCH v4 2/5] hwmon: (pmbus) Add support for Maxim Guenter Roeck
2011-02-17 19:00   ` [PATCH v4 2/5] hwmon: (pmbus) Add support for Maxim MAX8688 Guenter Roeck
2011-02-25 20:24   ` [lm-sensors] [PATCH v4 2/5] hwmon: (pmbus) Add support for Jonathan Cameron
2011-02-25 20:24     ` [PATCH v4 2/5] hwmon: (pmbus) Add support for Maxim MAX8688 Jonathan Cameron
2011-02-25 21:46     ` [lm-sensors] [PATCH v4 2/5] hwmon: (pmbus) Add support for Guenter Roeck
2011-02-25 21:46       ` [PATCH v4 2/5] hwmon: (pmbus) Add support for Maxim MAX8688 Guenter Roeck
2011-02-17 19:00 ` [lm-sensors] [PATCH v4 3/5] hwmon: (pmbus) Add support for Maxim Guenter Roeck
2011-02-17 19:00   ` [PATCH v4 3/5] hwmon: (pmbus) Add support for Maxim MAX16064 Guenter Roeck
2011-02-25 20:26   ` [lm-sensors] [PATCH v4 3/5] hwmon: (pmbus) Add support for Jonathan Cameron
2011-02-25 20:26     ` [PATCH v4 3/5] hwmon: (pmbus) Add support for Maxim MAX16064 Jonathan Cameron
2011-02-25 21:42     ` [lm-sensors] [PATCH v4 3/5] hwmon: (pmbus) Add support for Guenter Roeck
2011-02-25 21:42       ` [PATCH v4 3/5] hwmon: (pmbus) Add support for Maxim MAX16064 Guenter Roeck
2011-02-17 19:00 ` [lm-sensors] [PATCH v4 4/5] hwmon: (pmbus) Add support for Maxim Guenter Roeck
2011-02-17 19:00   ` [PATCH v4 4/5] hwmon: (pmbus) Add support for Maxim MAX34440/MAX34441 Guenter Roeck
2011-02-25 20:42   ` [lm-sensors] [PATCH v4 4/5] hwmon: (pmbus) Add support for Jonathan Cameron
2011-02-25 20:42     ` [PATCH v4 4/5] hwmon: (pmbus) Add support for Maxim MAX34440/MAX34441 Jonathan Cameron
2011-02-25 21:32     ` [lm-sensors] [PATCH v4 4/5] hwmon: (pmbus) Add support for Guenter Roeck
2011-02-25 21:32       ` [PATCH v4 4/5] hwmon: (pmbus) Add support for Maxim MAX34440/MAX34441 Guenter Roeck
2011-02-26 10:43       ` Jonathan Cameron [this message]
2011-02-26 10:43         ` Jonathan Cameron
2011-02-26 15:21         ` [lm-sensors] [PATCH v4 4/5] hwmon: (pmbus) Add support for Guenter Roeck
2011-02-26 15:21           ` [PATCH v4 4/5] hwmon: (pmbus) Add support for Maxim MAX34440/MAX34441 Guenter Roeck
2011-02-17 19:00 ` [lm-sensors] [PATCH v4 5/5] hwmon: pmbus driver documentation Guenter Roeck
2011-02-17 19:00   ` Guenter Roeck
2011-02-25 20:45   ` [lm-sensors] " Jonathan Cameron
2011-02-25 20:45     ` Jonathan Cameron

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=4D68D965.7040307@cam.ac.uk \
    --to=jic23@cam.ac.uk \
    --cc=gschnorr@cisco.com \
    --cc=guenter.roeck@ericsson.com \
    --cc=kernel@jic23.retrosnub.co.uk \
    --cc=khali@linux-fr.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lm-sensors@lm-sensors.org \
    --cc=rdunlap@xenotime.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.