LM83 driver

LM83 driver

[Jean Delvare]

Hi Zoleg,

I finished writting the driver you requested. Note that I don't own a
LM83 myself, so it is *completely untested*. It compiles, but I can't
promise much more. This is my first driver, so expect to encounter some
problems before it works.

You'd be welcome to checkout i2c (lk2-4 branch) and lm_sensors2 CVS and
give my driver a try :)

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

LM83 driver

[Jean Delvare]

> > I will, but I prefer to make sure the driver works OK before that.
> > And also, it is a minimalistic driver for now (doesn't handle
> > alarms, alarms mask and so on) so maybe it would be better to
> > enhance it on the 2.4 side and port it later to 2.5 than the
> > contrary.
> 
> Why?  2.5 can have minimal drivers :)

OK, I agree. I can port my driver to 2.5 before adding new features,
you're right.

> And it will get better coverage than the sensors cvs tree I bet...

Well, the CVS tree is to become release 2.8.0 soon, so the driver should
have a chance to get tested. Second, I don't have a working 2.5 kernel
here, so I wouldn't be able to make sure my driver even compiles (not to
say loads). I'll try to build a 2.5 kernel on one of my machines. Is
there anything special required (gcc, binutils, modutils?) Last time I
tried 2.5 I couldn't even compile it (I even wonder if I managed
configuring it at all.)

What I really would like right now is a preliminary test of my driver.
Zoleg, do you read us? After all, you're the one who asked for the
driver, so it would be only fair that you would test it now :)

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

LM83 driver

[Greg KH]

On Thu, Jul 03, 2003 at 08:34:31PM +0200, Jean Delvare wrote:
> 
> > > I finished writting the driver you requested. Note that I don't own
> > > a LM83 myself, so it is *completely untested*. It compiles, but I
> > > can't promise much more. This is my first driver, so expect to
> > > encounter some problems before it works.
> > 
> > Care to make a 2.5 patch too?  :)
> 
> I will, but I prefer to make sure the driver works OK before that. And
> also, it is a minimalistic driver for now (doesn't handle alarms, alarms
> mask and so on) so maybe it would be better to enhance it on the 2.4
> side and port it later to 2.5 than the contrary.

Why?  2.5 can have minimal drivers :)

And it will get better coverage than the sensors cvs tree I bet...

thanks,

greg k-h

LM83 driver

[Greg KH]

On Thu, Jul 03, 2003 at 07:45:58PM +0200, Jean Delvare wrote:
> 
> Hi Zoleg,
> 
> I finished writting the driver you requested. Note that I don't own a
> LM83 myself, so it is *completely untested*. It compiles, but I can't
> promise much more. This is my first driver, so expect to encounter some
> problems before it works.
> 
> You'd be welcome to checkout i2c (lk2-4 branch) and lm_sensors2 CVS and
> give my driver a try :)

Care to make a 2.5 patch too?  :)

greg k-h

LM83 driver

[Jean Delvare]

If anyone could take a look at my code for the LM83, I'd appreciate it.
This is my first driver, as you must know. I followed the guidelines in
doc/developers/new_drivers, tried to follow the coding standards, but I
may have missed a few things.

I propose the following changes to new_drivers file:

- Change the recommended driver to use as a template. The writer should
use the driver that is the more similar to the chip he is writing a
driver for. I personally used much more the lm75 and adm1021 drivers
than the recommended lm78. And actually, using two different drivers as
templates was great.

- Don't ask for testing with 2.2 kernels! Maybe we should add a line
about sending a 2.5 version (if possible) to Greg KH instead?

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

LM83 driver

[Jean Delvare]

> > I finished writting the driver you requested. Note that I don't own
> > a LM83 myself, so it is *completely untested*. It compiles, but I
> > can't promise much more. This is my first driver, so expect to
> > encounter some problems before it works.
> 
> Care to make a 2.5 patch too?  :)

I will, but I prefer to make sure the driver works OK before that. And
also, it is a minimalistic driver for now (doesn't handle alarms, alarms
mask and so on) so maybe it would be better to enhance it on the 2.4
side and port it later to 2.5 than the contrary.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

LM83 driver

[Philip Pokorny]

For no longer than the /proc standard is going to last (sysfs anyone) I think 
it would be much better to have a "dummy" value.

:v)

Mark D. Studebaker wrote:

> looks good.
> A couple of things I may have done differently -
> but that doesn't make the way you did it wrong :)
> 
> - You could have done one /proc callback function rather than 4
> - This is our first chip without two limits per sensor. To maintain our
>   /proc standard for temps we would need a 'dummy' second value between
>   the high limit and the reading. But if National did it, others will too,
>   so probably better to add to our /proc standard to say it could
>   be two values instead of three. Interesting.
> 
> 
> Jean Delvare wrote:
> 
>> If anyone could take a look at my code for the LM83, I'd appreciate it.
>> This is my first driver, as you must know. I followed the guidelines in
>> doc/developers/new_drivers, tried to follow the coding standards, but I
>> may have missed a few things.
>>
>> I propose the following changes to new_drivers file:
>>
>> - Change the recommended driver to use as a template. The writer should
>> use the driver that is the more similar to the chip he is writing a
>> driver for. I personally used much more the lm75 and adm1021 drivers
>> than the recommended lm78. And actually, using two different drivers as
>> templates was great.
>>
>> - Don't ask for testing with 2.2 kernels! Maybe we should add a line
>> about sending a 2.5 version (if possible) to Greg KH instead?
>>
> 

LM83 driver

[Jean Delvare]

> For no longer than the /proc standard is going to last (sysfs anyone)
> I think it would be much better to have a "dummy" value.

Why that? Our structures are flexible enough to support that, libsensors
supports it, sensors supports it. In which case would the dummy value
help? I just can't see any.

As for the procfs vs. sysfs thing, it is unrelated IMHO. If we are to
allow 2-values temperature files for sysfs, we can as well do so for
procfs. If anything needs to be changed to support this (I still don't
see what yet), it will have to for at least sysfs so it maybe won't even
notice it's the same for some recent procfs drivers.

Of course, adding a dummy value is easy and we already have at least one
driver that does it - adm1021 for lm84 - but I don't like adding
constraints when I feel they will only slow us down on our path to
progress. Letting the driver choose how many temperature values it wants
to repport is a good idea, methinks. One could imagine drivers only
repporting the current value, or max and current (this is the case for
my lm83 driver), or over, hyst and current as most of our drivers do.
But there are other possibilities, such as max, min, current (I think we
have some drivers that do that) or max, hystmax, min, hystmin and
current resulting in a 5-value file. The same could apply to other
measures such as in's and fan's.

The fact that we try to have a common base for all sensor drivers
doesn't mean we must ignore chips specificity. Sadly, it looks like this
is what we tend to do when a single drivers support many, many chips. I
think that's something we should think about. After all, that's the idea
Unix is living with for decades (do one thing and do it well).

Comments welcome, of course.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

LM83 driver

[Greg KH]

On Sat, Jul 05, 2003 at 07:57:46PM +0200, Jean Delvare wrote:
> 
> - Maybe we should add a line about sending a 2.5 version (if possible)
> to Greg KH instead?

And to the list, the more eyes, the better.

thanks,

greg k-h

LM83 driver

[Jean Delvare]

> - You could have done one /proc callback function rather than 4

Hm, how do I know who's calling me then? Using ctl_name and the
LM83_SYSCTL_* constants defined earlier? I don't exactly see the
interest yet. Though the functions are similar, most of the code really
depend on the sensor, so I don't think we'll win in clarity nor code
size. But maybe it's just me. Where could I look at a similar situation
in our code?

Anyway, this is left for after the release.

> - This is our first chip without two limits per sensor. To maintain
> our /proc standard for temps we would need a 'dummy' second value
> between the high limit and the reading. But if National did it,
> others will too, so probably better to add to our /proc standard to
> say it could be two values instead of three. Interesting.

At least the LM84 (in our adm1021 driver) and the LM82 (support to be
added to my LM83 driver) do the same. Probably some others do. I
couldn't see any reason for adding dummy values. It's probably clearer
to the user that way. When one sees only 2 values in the temp* files,
he/she'll understand that there is only one limit, which has to be a
high one. Using a dummy value will make him/her wonder why he/she can't
set it. Since our libsensors interface is flexible enough to allow this,
let's do it. What's more, we may have sensors with 3 or 4 limits someday
(min, max, hyst-min and hyst-max) or even more (if they differenciate
max and critical for example) and we will want to have them in our
files. We can also imagine sensors that report a temperature but don't
know about limits, thus having a single value. As long as we keep things
logical (current temperature being the last value, for example) and that
a given set of values is always ordered the same way, why wouldn't we
doing it?

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

LM83 driver

[Mark D. Studebaker]

looks good.
A couple of things I may have done differently -
but that doesn't make the way you did it wrong :)

- You could have done one /proc callback function rather than 4
- This is our first chip without two limits per sensor. To maintain our
   /proc standard for temps we would need a 'dummy' second value between
   the high limit and the reading. But if National did it, others will too,
   so probably better to add to our /proc standard to say it could
   be two values instead of three. Interesting.


Jean Delvare wrote:
> If anyone could take a look at my code for the LM83, I'd appreciate it.
> This is my first driver, as you must know. I followed the guidelines in
> doc/developers/new_drivers, tried to follow the coding standards, but I
> may have missed a few things.
> 
> I propose the following changes to new_drivers file:
> 
> - Change the recommended driver to use as a template. The writer should
> use the driver that is the more similar to the chip he is writing a
> driver for. I personally used much more the lm75 and adm1021 drivers
> than the recommended lm78. And actually, using two different drivers as
> templates was great.
> 
> - Don't ask for testing with 2.2 kernels! Maybe we should add a line
> about sending a 2.5 version (if possible) to Greg KH instead?
> 

LM83 driver

[Jean Delvare]

Hi Zoleg,

Three weeks ago, you requested a driver for the LM83 you say you had on
your motherboard. I wrote the driver and told you about it, but we never
heard about you again. Why that? It's not very polite to you to make me
work for you and ignore me once the job is done...

Nevermind, I would need your help now. First I would like you to try the
driver. It's included in our latest release (2.8.0), or you can use CVS
for a slightly improved version. I'd really like to know if the chip is
detected by our sensors-detect script, and how the driver behaves.

Second, I would like you to unload the lm83 driver (rmmod lm83) and dump
the contents of your LM83 chip. You will need to find at which address
it is located (sensors-detect should tell you) and run the following
command:
  i2cdump 0 0x18
(replace 18 with the address given by sensors-detect; if the chip is
located on a second I2C bus, replace 0 with 1).

Thanks.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

LM83 driver

[Mark D. Studebaker]

Jean Delvare wrote:
>>- You could have done one /proc callback function rather than 4
> 
> 
> Hm, how do I know who's calling me then? Using ctl_name and the
> LM83_SYSCTL_* constants defined earlier? I don't exactly see the
> interest yet. Though the functions are similar, most of the code really
> depend on the sensor, so I don't think we'll win in clarity nor code
> size. But maybe it's just me. Where could I look at a similar situation
> in our code?
> 


See w83781d_pwm() in w83781d.c which uses subtraction to convert the SYSCTL number
to PWM number, and the W83781D_REG_PWM macro which indexes into a static array of
register numbers.

LM83 driver

[Jean Delvare]

> > Hm, how do I know who's calling me then? Using ctl_name and the
> > LM83_SYSCTL_* constants defined earlier? I don't exactly see the
> > interest yet. Though the functions are similar, most of the code
> > really depend on the sensor, so I don't think we'll win in clarity
> > nor code size. But maybe it's just me. Where could I look at a
> > similar situation in our code?
> 
> See w83781d_pwm() in w83781d.c which uses subtraction to convert the
> SYSCTL number to PWM number, and the W83781D_REG_PWM macro which
> indexes into a static array of register numbers.

OK, I've done that. Looks better for sure, thanks for the tip.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

LM83 driver

[Jean Delvare]

> Of course with libsensors apps you don't care about /proc standards.
> 
> The benefit is in writing simple non-libsensors scripts, etc.
> (see our new prog/pwm/pwmconfig) which are only feasible if we follow
> consistent standards for /proc.

I read that script and fancontrol too, and I admit fancontrol wouldn't
work if there were 2-values temp files. It's really easy to fix though.
Just replace "cut -d' ' -f3" with "sed -e 's/^.* \(.*\)$/\1/'" and it's
done (I think awk would be better to do that, but unfortunately I don't
know awk at all).

Having 2-temp files (or 1-temp, or 4-temp) causes IMHO far less problems
than other differences we already have. This can always be workarounded
in scripts, as long as the first temp is the high limit, and the last
one is the current value (and my driver respects that). The fact that
some drivers output values that are *not* the real temps and that a
conversion formula is required is much more trouble, and that's the way
our drivers work however (or what would be sensors.conf for?) so the
non-libsensors scripts already have to handle that.

Maybe there will be some script to fix if we decide to allow
non-3-values temperature files, but I really think it will always be
easy and it makes the drivers easier to write, maintain and in some
cases even use.

-- 
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

LM83 driver

[Mark D. Studebaker]

Of course with libsensors apps you don't care about /proc standards.

The benefit is in writing simple non-libsensors scripts, etc.
(see our new prog/pwm/pwmconfig) which are only feasible if we follow
consistent standards for /proc.

Jean Delvare wrote:
>>For no longer than the /proc standard is going to last (sysfs anyone)
>>I think it would be much better to have a "dummy" value.
> 
> 
> Why that? Our structures are flexible enough to support that, libsensors
> supports it, sensors supports it. In which case would the dummy value
> help? I just can't see any.
> 
> As for the procfs vs. sysfs thing, it is unrelated IMHO. If we are to
> allow 2-values temperature files for sysfs, we can as well do so for
> procfs. If anything needs to be changed to support this (I still don't
> see what yet), it will have to for at least sysfs so it maybe won't even
> notice it's the same for some recent procfs drivers.
> 
> Of course, adding a dummy value is easy and we already have at least one
> driver that does it - adm1021 for lm84 - but I don't like adding
> constraints when I feel they will only slow us down on our path to
> progress. Letting the driver choose how many temperature values it wants
> to repport is a good idea, methinks. One could imagine drivers only
> repporting the current value, or max and current (this is the case for
> my lm83 driver), or over, hyst and current as most of our drivers do.
> But there are other possibilities, such as max, min, current (I think we
> have some drivers that do that) or max, hystmax, min, hystmin and
> current resulting in a 5-value file. The same could apply to other
> measures such as in's and fan's.
> 
> The fact that we try to have a common base for all sensor drivers
> doesn't mean we must ignore chips specificity. Sadly, it looks like this
> is what we tend to do when a single drivers support many, many chips. I
> think that's something we should think about. After all, that's the idea
> Unix is living with for decades (do one thing and do it well).
> 
> Comments welcome, of course.
> 

LM83 driver

[ZOleg Matrix]

Hello Jean,

Thursday, July 24, 2003, 1:09:58 AM, you wrote:


JD> Hi Zoleg,

JD> Three weeks ago, you requested a driver for the LM83 you say you had on
JD> your motherboard. I wrote the driver and told you about it, but we never
JD> heard about you again. Why that? It's not very polite to you to make me
JD> work for you and ignore me once the job is done...

JD> Nevermind, I would need your help now. First I would like you to try the
JD> driver. It's included in our latest release (2.8.0), or you can use CVS
JD> for a slightly improved version. I'd really like to know if the chip is
JD> detected by our sensors-detect script, and how the driver behaves.

JD> Second, I would like you to unload the lm83 driver (rmmod lm83) and dump
JD> the contents of your LM83 chip. You will need to find at which address
JD> it is located (sensors-detect should tell you) and run the following
JD> command:
JD>   i2cdump 0 0x18
JD> (replace 18 with the address given by sensors-detect; if the chip is
JD> located on a second I2C bus, replace 0 with 1).

JD> Thanks.


Excuse that did not leave on communication, but I was sick and have just come to work.
Today I shall try to test the new version of the driver and to send you results.

-- 
Best regards,
 ZOleg                            mailto:zoleg@matrixc.ru