Thread (2 messages) 2 messages, 2 authors, 2010-08-14

Re: [lm-sensors] [PATCH] hwmon: (tmp421) Add nfactor support (2nd attempt)

From: Jean Delvare <hidden>
Date: 2010-08-14 20:00:14

Hi Andre, Guenter, Jeff,

On Wed, 21 Jul 2010 21:46:50 +0200, Andre Prendel wrote:
On Tue, Jul 20, 2010 at 08:59:52AM -0700, Guenter Roeck wrote:
quoted
On Tue, Jul 20, 2010 at 11:09:53AM -0400, Andre Prendel wrote:
quoted
On Thu, May 20, 2010 at 09:35:56PM +0200, Andre Prendel wrote:
quoted
On Thu, May 20, 2010 at 03:07:05PM -0400, Jeff Angielski wrote:
quoted
In any event, here it is again:
Acked-by: Andre Prendel <redacted>
Hi Jeff,

I'de suggest to resend the patch with my acked-by to the lm-sensors list and
Andrew Morton. It looks like Jean is too busy at the moment.

Regards,
Andre
 
quoted
quoted
quoted
From 9acd29ff48c64e58a7f5cdb888c86e737c56281c Mon Sep 17 00:00:00 2001
From: Jeff Angielski <redacted>
Date: Mon, 10 May 2010 10:26:34 -0400
Subject: [PATCH] hwmon: (tmp421) Add nfactor support

Add support for reading and writing the n-factor correction
registers.  This is needed to compensate for the characteristics
of a particular sensor hanging off of the remote channels.
My concerns with this approach are 

1) It changes the sysfs abi. libsensors won't support it. It opens up
   a can of worms with everyone starting to put chip-specific extensions
   into the ABI. If such an extension has to be made, it should be a) really necessary
   and b) a generic extension which applies to all chips.
A chip-specific extension can't be also generic. So we have to decide whether weaccept chip-specific extensions or not.
libsensors 3 will never support chip-specific extensions. We've been
there with libsensors 2, it was a nightmare, never again please.

This doesn't mean we can't have chip-specific extensions, and actually
some drivers have such extensions already. But as they are often
undocumented and bound to be replaced by standard implementations in
the future, nobody should rely on them. Which makes their usefulness
dubious.

This is the reason why I always insist on trying to define a standard
suitable for all chips before you think of adding a not-yet-supported
feature to a given driver.
quoted
2) My understanding is that value adjustments are supposed to be made via sensors.conf,
   and that values reported by the chip should always be 'raw', ie unadjusted
   values.

Instead of exporting n_adjust to the user via sysfs, it might make more sense 
to reset the correction factor to its default (if it was changed), and handle
required adjustments in sensors.conf.
Jeff, what do you think?
I'm not Jeff, but resetting the factors is a bad idea. The
BIOS/firmware might have set them up properly, so the default should be
to leave them untouched (as we do with every other setting.)
quoted
Even if Jean doesn't have time to handle the patch, you should at least get his Ack
for the ABI changes.
That was the intention of resending the patch to the lm-sensors list. It would
be a pity to lose Jeff's effort.
I'm not giving my ack for any non-standard feature, sorry.

-- 
Jean Delvare
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help