[lm-sensors] Need some guidance on adhering to the sysfs standards

[lm-sensors] Need some guidance on adhering to the sysfs standards

[George T. Joseph (development)]

I'm wrapping up the Andigilog driver and am having problems trying to
adhere to the sysfs auto_point definitions.

These chips have 2 temp points.  The low temp (auto_point1_temp) is
simple enough to map to the corresponding register.  The high point
(auto_point2_temp) isn't so easy because it's implemented as a range
with the low temp as a base.  Even worse, range is not an arbitrary
value; it's a value from a list of 16.  So if a user chooses 26 for a
low point and 50 for a high point, that's a range of 24 except that 24
isn't a valid range value.  The closest values are either 20 or 26.67.
Also, if the user sets the high point then changes the low point, the
high point will probably have to be changed because the new calculated
range might not be a valid value.

Auto_point_pwm is also an issue.  Where there are 4 temp zones, there
are only 3 pwm outputs and the assignment of pwm to zone can be changed
by the user or automatically by the chip.   

Finally, there's only 1 hysteresis value per zone, not 1 per auto_point
so should I do an auto_point1_temp_hyst and an auto_point2_temp_hyst and
have them both point to the same register or maybe a
auto_point_temp_hyst without an index?

I guess I have 4 options...

1) Don't implement auto_point functionality at all.  A bad choice I
believe.

2) Implement the logic to get as close as possible to the standard but
this is going to be very confusing to a user especially the range
business since changing 1 value can force change of others.  It's also
going to increase the complexity of the driver significantly.

3) Adhere to the standard where possible without creating the compound
relationships.  So what I'd do is...
 temp[1-4]_auto_point_temp_min
 temp[1-4]_auto_point_temp_range
 temp[1-4]_auto_point_temp_hyst
 pwm[1-3]_auto_channels_temp
 pwm[1-3]_auto_point_pwm_min
 pwm[1-3]_auto_point_pwm_max

4) Or I could just name them what they're named in the data sheet...

 temp[1-4]_fan_temp_limit
 temp[1-4]_range
 temp[1-4]_hyst
 pwm[1-3]_config
 pwm{1-3]_min
 pwm[1-3]_max


Thoughts?







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

Re: [lm-sensors] Need some guidance on adhering to the sysfs

[Jean Delvare]

Hi George,

On Thu, 12 Apr 2007 10:50:30 -0600, George T. Joseph \(development\) wrote:
> I'm wrapping up the Andigilog driver and am having problems trying to
> adhere to the sysfs auto_point definitions.
> 
> These chips have 2 temp points.  The low temp (auto_point1_temp) is
> simple enough to map to the corresponding register.  The high point
> (auto_point2_temp) isn't so easy because it's implemented as a range
> with the low temp as a base.  Even worse, range is not an arbitrary
> value; it's a value from a list of 16.  So if a user chooses 26 for a
> low point and 50 for a high point, that's a range of 24 except that 24
> isn't a valid range value.  The closest values are either 20 or 26.67.
> Also, if the user sets the high point then changes the low point, the
> high point will probably have to be changed because the new calculated
> range might not be a valid value.

Pick the nearest possible value for auto_point2_temp. If
auto_point1_temp is changed afterwards, indeed the range register
should be adjusted so that auto_point2_temp is preserved as much as
possible (principle of least surprise). I understand there will be some
loss, but that's better than ignoring the issue altogether. I do not
expect it to be a big deal in practice, as I think the users typically
set auto_point1 first.

> Auto_point_pwm is also an issue.  Where there are 4 temp zones, there
> are only 3 pwm outputs and the assignment of pwm to zone can be changed
> by the user or automatically by the chip.   

Automatically, based on what?
What is a "temp zone" for this device?

We have a mapping file (auto_channels) between PWM outputs and
temperature inputs which is supposed to address this issue.

> Finally, there's only 1 hysteresis value per zone, not 1 per auto_point
> so should I do an auto_point1_temp_hyst and an auto_point2_temp_hyst and
> have them both point to the same register or maybe a
> auto_point_temp_hyst without an index?

Make auto_point1_temp_hyst writable and auto_point2_temp_hyst read-only.

> I guess I have 4 options...
> 
> 1) Don't implement auto_point functionality at all.  A bad choice I
> believe.
> 
> 2) Implement the logic to get as close as possible to the standard but
> this is going to be very confusing to a user especially the range
> business since changing 1 value can force change of others.  It's also
> going to increase the complexity of the driver significantly.

Yes, please do this. It shouldn't be so hard. See my advice above.

Feel free to create a file under Documentation/hwmon for your driver
and document the limitations if you think the user might be confused.

> 3) Adhere to the standard where possible without creating the compound
> relationships.  So what I'd do is...
>  temp[1-4]_auto_point_temp_min
>  temp[1-4]_auto_point_temp_range
>  temp[1-4]_auto_point_temp_hyst
>  pwm[1-3]_auto_channels_temp
>  pwm[1-3]_auto_point_pwm_min
>  pwm[1-3]_auto_point_pwm_max
> 
> 4) Or I could just name them what they're named in the data sheet...
> 
>  temp[1-4]_fan_temp_limit
>  temp[1-4]_range
>  temp[1-4]_hyst
>  pwm[1-3]_config
>  pwm{1-3]_min
>  pwm[1-3]_max

This would be particularly confusing, as these names don't show the
relations between the items.

-- 
Jean Delvare

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

Re: [lm-sensors] Need some guidance on adhering to the sysfs

[George T. Joseph (development)]

Thanks Jean,

More comments in line...

george

> -----Original Message-----
> From: Jean Delvare [mailto:khali@linux-fr.org] 
> Sent: Thursday, April 19, 2007 8:48 AM
> To: George T. Joseph (development)
> Cc: lm-sensors@lm-sensors.org
> Subject: Re: [lm-sensors] Need some guidance on adhering to 
> the sysfs standards
> 
> Hi George,
> 
> On Thu, 12 Apr 2007 10:50:30 -0600, George T. Joseph 
> \(development\) wrote:
> > I'm wrapping up the Andigilog driver and am having problems 
> trying to
> > adhere to the sysfs auto_point definitions.
> > 
> > These chips have 2 temp points.  The low temp (auto_point1_temp) is
> > simple enough to map to the corresponding register.  The high point
> > (auto_point2_temp) isn't so easy because it's implemented as a range
> > with the low temp as a base.  Even worse, range is not an arbitrary
> > value; it's a value from a list of 16.  So if a user 
> chooses 26 for a
> > low point and 50 for a high point, that's a range of 24 
> except that 24
> > isn't a valid range value.  The closest values are either 
> 20 or 26.67.
> > Also, if the user sets the high point then changes the low 
> point, the
> > high point will probably have to be changed because the new 
> calculated
> > range might not be a valid value.
> 
> Pick the nearest possible value for auto_point2_temp. If
> auto_point1_temp is changed afterwards, indeed the range register
> should be adjusted so that auto_point2_temp is preserved as much as
> possible (principle of least surprise). I understand there 
> will be some
> loss, but that's better than ignoring the issue altogether. I do not
> expect it to be a big deal in practice, as I think the users typically
> set auto_point1 first.

OK, I can do that but rather than "nearest" point I'll have to go with
the lower range value because the higher range value could result in a
heat related failure.

> 
> > Auto_point_pwm is also an issue.  Where there are 4 temp 
> zones, there
> > are only 3 pwm outputs and the assignment of pwm to zone 
> can be changed
> > by the user or automatically by the chip.   
> 
> Automatically, based on what?
> What is a "temp zone" for this device?
> 
> We have a mapping file (auto_channels) between PWM outputs and
> temperature inputs which is supposed to address this issue.

The chip has 4 "zones".  Each zone can be configured from 7 sources
available: 2 remote diodes, the internal chip temp, and 4 PECI bus
temps.  I've created a temp[1-4]_source file for this.  There are 3 PWM
outputs, each of which can be controlled by a single zone, or by the
"hottest of zone 2 or 3",  "hottest of zone 1, 2, or 3", or "hottest of
zone 1, 2, 3, or 4".  "hottest" is determined in real time by the chip.
The actual zone controlling a pwm output at any point in time is found
in a separate register.  I have that exposed as
pwm[1-3]_auto_zone_assigned.   

Min and max pwms are associated to the pwm output itself while the temp
min and maxes are associated to the zones.  Hence the following is valid
and what I expose...
pwm[1-3]_auto_channels_temp
pwm[1-3]_auto_point[1-2]_pwm
temp[1-4]_auto_point[1-2]_temp
temp[1-4]_auto_point[1-2]_temp_hyst

Another little issue I just realized has to do with what happens to pwm
output when a temp is BELOW the minimum for the zone.  You can configure
the pwm output to be either off or run at the minimum set.  If you want
it to be off, you can't just set the min pwm to 0 because that would
imply that when the temp does rise to the min temp, the pwm slope would
start from 0.  That's not good because the fan might not even run until
the pwm duty cycle is above 50 or 75.  The behavior needed is that when
the temp is below the min, the fan is off, when the temp reaches the min
temp, the fan runs at the min pwm duty cycle then scales up as the temp
does.  So, I'll have to create a separate sysfs file for this control
and it's going to need a name.  Can you suggest one that means "turn the
pwm off if the temp is below the auto_point1_temp"?  
Maybe pwm[1-3]_auto_point1_pwm_off_if_below :)

> 
> > Finally, there's only 1 hysteresis value per zone, not 1 
> per auto_point
> > so should I do an auto_point1_temp_hyst and an 
> auto_point2_temp_hyst and
> > have them both point to the same register or maybe a
> > auto_point_temp_hyst without an index?
> 
> Make auto_point1_temp_hyst writable and auto_point2_temp_hyst 
> read-only.

Easy enough.

> 
> > I guess I have 4 options...
> > 
> > 1) Don't implement auto_point functionality at all.  A bad choice I
> > believe.
> > 
> > 2) Implement the logic to get as close as possible to the 
> standard but
> > this is going to be very confusing to a user especially the range
> > business since changing 1 value can force change of others. 
>  It's also
> > going to increase the complexity of the driver significantly.
> 
> Yes, please do this. It shouldn't be so hard. See my advice above.
> 
> Feel free to create a file under Documentation/hwmon for your driver
> and document the limitations if you think the user might be confused.

It's not difficult, just convoluted.  I have to synthesize sysfs files
from multiple registers which weren't designed to be related by the
manufacturer.

I have a documentation file ready but I'm afraid that rather than
describing the capabilities of the chip it's going to be mostly
explaining the fallout from shoehorning it into a shoe that's one size
to small. :)

> 
> > 3) Adhere to the standard where possible without creating 
> the compound
> > relationships.  So what I'd do is...
> >  temp[1-4]_auto_point_temp_min
> >  temp[1-4]_auto_point_temp_range
> >  temp[1-4]_auto_point_temp_hyst
> >  pwm[1-3]_auto_channels_temp
> >  pwm[1-3]_auto_point_pwm_min
> >  pwm[1-3]_auto_point_pwm_max
> > 
> > 4) Or I could just name them what they're named in the data sheet...
> > 
> >  temp[1-4]_fan_temp_limit
> >  temp[1-4]_range
> >  temp[1-4]_hyst
> >  pwm[1-3]_config
> >  pwm{1-3]_min
> >  pwm[1-3]_max
> 
> This would be particularly confusing, as these names don't show the
> relations between the items.
> 

Which is confusing, 3 or 4?  I agree that 4 might be but 3 is as
straightforward as you can get.

> -- 
> Jean Delvare
> 

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

Re: [lm-sensors] Need some guidance on adhering to the sysfs

[Juerg Haefliger]

George, Jean,

I'm struggling with the same issue for the DME1737 I'm currently
working on. This chip also features temp zones and "hottest of x,y,z"
PWM control. The current sysfs standard is not flexible enough to
handle these features, especially the combination of a single PWM
being controlled by multiple temp inputs and multiple PMWs being
controlled by the same temp input. I believe we need another layer of
mapping. I.e. temp->pwm is not sufficient, but rather temp->zone->pwm.

I therefore propose the add the following sysfs attributes to our standard:

zone[1-*]_auto_channels_temp   for temp-to-zone mapping
pwm[1-*]_auto_channels_zone   for zone-to-pwm mapping
zone[1-*]_auto_point[1-*]_temp   for zone temp auto points.

What do you guys think?

...juerg



On 4/19/07, George T. Joseph (development) <gtj.devel@peakin.com> wrote:
> Thanks Jean,
>
> More comments in line...
>
> george
>
> > -----Original Message-----
> > From: Jean Delvare [mailto:khali@linux-fr.org]
> > Sent: Thursday, April 19, 2007 8:48 AM
> > To: George T. Joseph (development)
> > Cc: lm-sensors@lm-sensors.org
> > Subject: Re: [lm-sensors] Need some guidance on adhering to
> > the sysfs standards
> >
> > Hi George,
> >
> > On Thu, 12 Apr 2007 10:50:30 -0600, George T. Joseph
> > \(development\) wrote:
> > > I'm wrapping up the Andigilog driver and am having problems
> > trying to
> > > adhere to the sysfs auto_point definitions.
> > >
> > > These chips have 2 temp points.  The low temp (auto_point1_temp) is
> > > simple enough to map to the corresponding register.  The high point
> > > (auto_point2_temp) isn't so easy because it's implemented as a range
> > > with the low temp as a base.  Even worse, range is not an arbitrary
> > > value; it's a value from a list of 16.  So if a user
> > chooses 26 for a
> > > low point and 50 for a high point, that's a range of 24
> > except that 24
> > > isn't a valid range value.  The closest values are either
> > 20 or 26.67.
> > > Also, if the user sets the high point then changes the low
> > point, the
> > > high point will probably have to be changed because the new
> > calculated
> > > range might not be a valid value.
> >
> > Pick the nearest possible value for auto_point2_temp. If
> > auto_point1_temp is changed afterwards, indeed the range register
> > should be adjusted so that auto_point2_temp is preserved as much as
> > possible (principle of least surprise). I understand there
> > will be some
> > loss, but that's better than ignoring the issue altogether. I do not
> > expect it to be a big deal in practice, as I think the users typically
> > set auto_point1 first.
>
> OK, I can do that but rather than "nearest" point I'll have to go with
> the lower range value because the higher range value could result in a
> heat related failure.
>
> >
> > > Auto_point_pwm is also an issue.  Where there are 4 temp
> > zones, there
> > > are only 3 pwm outputs and the assignment of pwm to zone
> > can be changed
> > > by the user or automatically by the chip.
> >
> > Automatically, based on what?
> > What is a "temp zone" for this device?
> >
> > We have a mapping file (auto_channels) between PWM outputs and
> > temperature inputs which is supposed to address this issue.
>
> The chip has 4 "zones".  Each zone can be configured from 7 sources
> available: 2 remote diodes, the internal chip temp, and 4 PECI bus
> temps.  I've created a temp[1-4]_source file for this.  There are 3 PWM
> outputs, each of which can be controlled by a single zone, or by the
> "hottest of zone 2 or 3",  "hottest of zone 1, 2, or 3", or "hottest of
> zone 1, 2, 3, or 4".  "hottest" is determined in real time by the chip.
> The actual zone controlling a pwm output at any point in time is found
> in a separate register.  I have that exposed as
> pwm[1-3]_auto_zone_assigned.
>
> Min and max pwms are associated to the pwm output itself while the temp
> min and maxes are associated to the zones.  Hence the following is valid
> and what I expose...
> pwm[1-3]_auto_channels_temp
> pwm[1-3]_auto_point[1-2]_pwm
> temp[1-4]_auto_point[1-2]_temp
> temp[1-4]_auto_point[1-2]_temp_hyst
>
> Another little issue I just realized has to do with what happens to pwm
> output when a temp is BELOW the minimum for the zone.  You can configure
> the pwm output to be either off or run at the minimum set.  If you want
> it to be off, you can't just set the min pwm to 0 because that would
> imply that when the temp does rise to the min temp, the pwm slope would
> start from 0.  That's not good because the fan might not even run until
> the pwm duty cycle is above 50 or 75.  The behavior needed is that when
> the temp is below the min, the fan is off, when the temp reaches the min
> temp, the fan runs at the min pwm duty cycle then scales up as the temp
> does.  So, I'll have to create a separate sysfs file for this control
> and it's going to need a name.  Can you suggest one that means "turn the
> pwm off if the temp is below the auto_point1_temp"?
> Maybe pwm[1-3]_auto_point1_pwm_off_if_below :)
>
> >
> > > Finally, there's only 1 hysteresis value per zone, not 1
> > per auto_point
> > > so should I do an auto_point1_temp_hyst and an
> > auto_point2_temp_hyst and
> > > have them both point to the same register or maybe a
> > > auto_point_temp_hyst without an index?
> >
> > Make auto_point1_temp_hyst writable and auto_point2_temp_hyst
> > read-only.
>
> Easy enough.
>
> >
> > > I guess I have 4 options...
> > >
> > > 1) Don't implement auto_point functionality at all.  A bad choice I
> > > believe.
> > >
> > > 2) Implement the logic to get as close as possible to the
> > standard but
> > > this is going to be very confusing to a user especially the range
> > > business since changing 1 value can force change of others.
> >  It's also
> > > going to increase the complexity of the driver significantly.
> >
> > Yes, please do this. It shouldn't be so hard. See my advice above.
> >
> > Feel free to create a file under Documentation/hwmon for your driver
> > and document the limitations if you think the user might be confused.
>
> It's not difficult, just convoluted.  I have to synthesize sysfs files
> from multiple registers which weren't designed to be related by the
> manufacturer.
>
> I have a documentation file ready but I'm afraid that rather than
> describing the capabilities of the chip it's going to be mostly
> explaining the fallout from shoehorning it into a shoe that's one size
> to small. :)
>
> >
> > > 3) Adhere to the standard where possible without creating
> > the compound
> > > relationships.  So what I'd do is...
> > >  temp[1-4]_auto_point_temp_min
> > >  temp[1-4]_auto_point_temp_range
> > >  temp[1-4]_auto_point_temp_hyst
> > >  pwm[1-3]_auto_channels_temp
> > >  pwm[1-3]_auto_point_pwm_min
> > >  pwm[1-3]_auto_point_pwm_max
> > >
> > > 4) Or I could just name them what they're named in the data sheet...
> > >
> > >  temp[1-4]_fan_temp_limit
> > >  temp[1-4]_range
> > >  temp[1-4]_hyst
> > >  pwm[1-3]_config
> > >  pwm{1-3]_min
> > >  pwm[1-3]_max
> >
> > This would be particularly confusing, as these names don't show the
> > relations between the items.
> >
>
> Which is confusing, 3 or 4?  I agree that 4 might be but 3 is as
> straightforward as you can get.
>
> > --
> > Jean Delvare
> >
>
> _______________________________________________
> lm-sensors mailing list
> lm-sensors@lm-sensors.org
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
>

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

Re: [lm-sensors] Need some guidance on adhering to the sysfs

[George T. Joseph (development)]

I like it.  It fits. 

> -----Original Message-----
> From: Juerg Haefliger [mailto:juergh@gmail.com] 
> Sent: Thursday, April 19, 2007 1:04 PM
> To: George T. Joseph (development)
> Cc: Jean Delvare; lm-sensors@lm-sensors.org
> Subject: Re: [lm-sensors] Need some guidance on adhering to 
> the sysfs standards
> 
> George, Jean,
> 
> I'm struggling with the same issue for the DME1737 I'm currently
> working on. This chip also features temp zones and "hottest of x,y,z"
> PWM control. The current sysfs standard is not flexible enough to
> handle these features, especially the combination of a single PWM
> being controlled by multiple temp inputs and multiple PMWs being
> controlled by the same temp input. I believe we need another layer of
> mapping. I.e. temp->pwm is not sufficient, but rather temp->zone->pwm.
> 
> I therefore propose the add the following sysfs attributes to 
> our standard:
> 
> zone[1-*]_auto_channels_temp   for temp-to-zone mapping
> pwm[1-*]_auto_channels_zone   for zone-to-pwm mapping
> zone[1-*]_auto_point[1-*]_temp   for zone temp auto points.
> 
> What do you guys think?
> 
> ...juerg
> 
> 
> 
> On 4/19/07, George T. Joseph (development) 
> <gtj.devel@peakin.com> wrote:
> > Thanks Jean,
> >
> > More comments in line...
> >
> > george
> >
> > > -----Original Message-----
> > > From: Jean Delvare [mailto:khali@linux-fr.org]
> > > Sent: Thursday, April 19, 2007 8:48 AM
> > > To: George T. Joseph (development)
> > > Cc: lm-sensors@lm-sensors.org
> > > Subject: Re: [lm-sensors] Need some guidance on adhering to
> > > the sysfs standards
> > >
> > > Hi George,
> > >
> > > On Thu, 12 Apr 2007 10:50:30 -0600, George T. Joseph
> > > \(development\) wrote:
> > > > I'm wrapping up the Andigilog driver and am having problems
> > > trying to
> > > > adhere to the sysfs auto_point definitions.
> > > >
> > > > These chips have 2 temp points.  The low temp 
> (auto_point1_temp) is
> > > > simple enough to map to the corresponding register.  
> The high point
> > > > (auto_point2_temp) isn't so easy because it's 
> implemented as a range
> > > > with the low temp as a base.  Even worse, range is not 
> an arbitrary
> > > > value; it's a value from a list of 16.  So if a user
> > > chooses 26 for a
> > > > low point and 50 for a high point, that's a range of 24
> > > except that 24
> > > > isn't a valid range value.  The closest values are either
> > > 20 or 26.67.
> > > > Also, if the user sets the high point then changes the low
> > > point, the
> > > > high point will probably have to be changed because the new
> > > calculated
> > > > range might not be a valid value.
> > >
> > > Pick the nearest possible value for auto_point2_temp. If
> > > auto_point1_temp is changed afterwards, indeed the range register
> > > should be adjusted so that auto_point2_temp is preserved 
> as much as
> > > possible (principle of least surprise). I understand there
> > > will be some
> > > loss, but that's better than ignoring the issue 
> altogether. I do not
> > > expect it to be a big deal in practice, as I think the 
> users typically
> > > set auto_point1 first.
> >
> > OK, I can do that but rather than "nearest" point I'll have 
> to go with
> > the lower range value because the higher range value could 
> result in a
> > heat related failure.
> >
> > >
> > > > Auto_point_pwm is also an issue.  Where there are 4 temp
> > > zones, there
> > > > are only 3 pwm outputs and the assignment of pwm to zone
> > > can be changed
> > > > by the user or automatically by the chip.
> > >
> > > Automatically, based on what?
> > > What is a "temp zone" for this device?
> > >
> > > We have a mapping file (auto_channels) between PWM outputs and
> > > temperature inputs which is supposed to address this issue.
> >
> > The chip has 4 "zones".  Each zone can be configured from 7 sources
> > available: 2 remote diodes, the internal chip temp, and 4 PECI bus
> > temps.  I've created a temp[1-4]_source file for this.  
> There are 3 PWM
> > outputs, each of which can be controlled by a single zone, or by the
> > "hottest of zone 2 or 3",  "hottest of zone 1, 2, or 3", or 
> "hottest of
> > zone 1, 2, 3, or 4".  "hottest" is determined in real time 
> by the chip.
> > The actual zone controlling a pwm output at any point in 
> time is found
> > in a separate register.  I have that exposed as
> > pwm[1-3]_auto_zone_assigned.
> >
> > Min and max pwms are associated to the pwm output itself 
> while the temp
> > min and maxes are associated to the zones.  Hence the 
> following is valid
> > and what I expose...
> > pwm[1-3]_auto_channels_temp
> > pwm[1-3]_auto_point[1-2]_pwm
> > temp[1-4]_auto_point[1-2]_temp
> > temp[1-4]_auto_point[1-2]_temp_hyst
> >
> > Another little issue I just realized has to do with what 
> happens to pwm
> > output when a temp is BELOW the minimum for the zone.  You 
> can configure
> > the pwm output to be either off or run at the minimum set.  
> If you want
> > it to be off, you can't just set the min pwm to 0 because that would
> > imply that when the temp does rise to the min temp, the pwm 
> slope would
> > start from 0.  That's not good because the fan might not 
> even run until
> > the pwm duty cycle is above 50 or 75.  The behavior needed 
> is that when
> > the temp is below the min, the fan is off, when the temp 
> reaches the min
> > temp, the fan runs at the min pwm duty cycle then scales up 
> as the temp
> > does.  So, I'll have to create a separate sysfs file for 
> this control
> > and it's going to need a name.  Can you suggest one that 
> means "turn the
> > pwm off if the temp is below the auto_point1_temp"?
> > Maybe pwm[1-3]_auto_point1_pwm_off_if_below :)
> >
> > >
> > > > Finally, there's only 1 hysteresis value per zone, not 1
> > > per auto_point
> > > > so should I do an auto_point1_temp_hyst and an
> > > auto_point2_temp_hyst and
> > > > have them both point to the same register or maybe a
> > > > auto_point_temp_hyst without an index?
> > >
> > > Make auto_point1_temp_hyst writable and auto_point2_temp_hyst
> > > read-only.
> >
> > Easy enough.
> >
> > >
> > > > I guess I have 4 options...
> > > >
> > > > 1) Don't implement auto_point functionality at all.  A 
> bad choice I
> > > > believe.
> > > >
> > > > 2) Implement the logic to get as close as possible to the
> > > standard but
> > > > this is going to be very confusing to a user especially 
> the range
> > > > business since changing 1 value can force change of others.
> > >  It's also
> > > > going to increase the complexity of the driver significantly.
> > >
> > > Yes, please do this. It shouldn't be so hard. See my advice above.
> > >
> > > Feel free to create a file under Documentation/hwmon for 
> your driver
> > > and document the limitations if you think the user might 
> be confused.
> >
> > It's not difficult, just convoluted.  I have to synthesize 
> sysfs files
> > from multiple registers which weren't designed to be related by the
> > manufacturer.
> >
> > I have a documentation file ready but I'm afraid that rather than
> > describing the capabilities of the chip it's going to be mostly
> > explaining the fallout from shoehorning it into a shoe 
> that's one size
> > to small. :)
> >
> > >
> > > > 3) Adhere to the standard where possible without creating
> > > the compound
> > > > relationships.  So what I'd do is...
> > > >  temp[1-4]_auto_point_temp_min
> > > >  temp[1-4]_auto_point_temp_range
> > > >  temp[1-4]_auto_point_temp_hyst
> > > >  pwm[1-3]_auto_channels_temp
> > > >  pwm[1-3]_auto_point_pwm_min
> > > >  pwm[1-3]_auto_point_pwm_max
> > > >
> > > > 4) Or I could just name them what they're named in the 
> data sheet...
> > > >
> > > >  temp[1-4]_fan_temp_limit
> > > >  temp[1-4]_range
> > > >  temp[1-4]_hyst
> > > >  pwm[1-3]_config
> > > >  pwm{1-3]_min
> > > >  pwm[1-3]_max
> > >
> > > This would be particularly confusing, as these names 
> don't show the
> > > relations between the items.
> > >
> >
> > Which is confusing, 3 or 4?  I agree that 4 might be but 3 is as
> > straightforward as you can get.
> >
> > > --
> > > Jean Delvare
> > >
> >
> > _______________________________________________
> > lm-sensors mailing list
> > lm-sensors@lm-sensors.org
> > http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
> >
> 

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

Re: [lm-sensors] Need some guidance on adhering to the sysfs

[Jean Delvare]

Hi Juerg,

On Thu, 19 Apr 2007 12:04:28 -0700, Juerg Haefliger wrote:
> George, Jean,
> 
> I'm struggling with the same issue for the DME1737 I'm currently
> working on. This chip also features temp zones and "hottest of x,y,z"
> PWM control. The current sysfs standard is not flexible enough to
> handle these features, especially the combination of a single PWM
> being controlled by multiple temp inputs and multiple PMWs being
> controlled by the same temp input. I believe we need another layer of
> mapping. I.e. temp->pwm is not sufficient, but rather temp->zone->pwm.
> 
> I therefore propose the add the following sysfs attributes to our standard:
> 
> zone[1-*]_auto_channels_temp   for temp-to-zone mapping
> pwm[1-*]_auto_channels_zone   for zone-to-pwm mapping
> zone[1-*]_auto_point[1-*]_temp   for zone temp auto points.

I don't see what value it adds compared to what we currently have.

We have pwm[1-*]_auto_channels_temp, which is a bit vector. We have one
file per PWM, one bit per temperature channel, so all in all we have a
Npwm x Ntemp matrix, or N-N relation between PWM and temperatures. This
already allows us to handle cases such as "the hottest of tempA and
tempB control pwmC" or "tempD controls pwmE and pwmF".

You propose to add the concept of zone. According to the above, each
zone could include any temperature channel, so we have a N-N relation
between zones and temperatures. Then you express another N-N relation
between PWM channels and zones. As far as I can see, this results in a
N-N relation between temperatures and PWM, just expressed differently.
Am I missing something? What do you think it would let us express,
which the current model doesn't?

-- 
Jean Delvare

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

Re: [lm-sensors] Need some guidance on adhering to the sysfs

[Mark M. Hoffman]

Hi guys:

> On Thu, 19 Apr 2007 12:04:28 -0700, Juerg Haefliger wrote:
> > I'm struggling with the same issue for the DME1737 I'm currently
> > working on. This chip also features temp zones and "hottest of x,y,z"
> > PWM control. The current sysfs standard is not flexible enough to
> > handle these features, especially the combination of a single PWM
> > being controlled by multiple temp inputs and multiple PMWs being
> > controlled by the same temp input. I believe we need another layer of
> > mapping. I.e. temp->pwm is not sufficient, but rather temp->zone->pwm.
> > 
> > I therefore propose the add the following sysfs attributes to our standard:
> > 
> > zone[1-*]_auto_channels_temp   for temp-to-zone mapping
> > pwm[1-*]_auto_channels_zone   for zone-to-pwm mapping
> > zone[1-*]_auto_point[1-*]_temp   for zone temp auto points.

* Jean Delvare <khali@linux-fr.org> [2007-04-22 17:29:04 +0200]:
> I don't see what value it adds compared to what we currently have.
> 
> We have pwm[1-*]_auto_channels_temp, which is a bit vector. We have one
> file per PWM, one bit per temperature channel, so all in all we have a
> Npwm x Ntemp matrix, or N-N relation between PWM and temperatures. This
> already allows us to handle cases such as "the hottest of tempA and
> tempB control pwmC" or "tempD controls pwmE and pwmF".
> 
> You propose to add the concept of zone. According to the above, each
> zone could include any temperature channel, so we have a N-N relation
> between zones and temperatures. Then you express another N-N relation
> between PWM channels and zones. As far as I can see, this results in a
> N-N relation between temperatures and PWM, just expressed differently.
> Am I missing something? What do you think it would let us express,
> which the current model doesn't?

I was never a big fan of the pwm_auto standard interface in the first place.
IMHO, the various chips implement automatic fan control in so many different
ways that the abstraction is bound to be mis-aligned for most chips, no matter
how we model it.

Or if not, then the abstraction would necessarily have so many options that it
hardly even counts as an abstraction.

I don't see it in Documentation/hwmon/sysfs-interface, but IIRC it was agreed
that the pwm_auto standard interface was optional.  So, a driver may support
that interface as best it can, OR it can support one which describes its actual
capabilities precisely (so long as there is no collision of sysfs names).  It
is important to note that libsensors has no pwm_auto support anyway, and none
is planned.

So I agree with Jean that we shouldn't extend the "standard" interface.  If
you feel that the standard interface is insufficient for your hardware, then
go ahead and create something completely different.  If the chip is "close"
to fitting the standard interface, I would prefer you support that as best
you can.

Regards,

-- 
Mark M. Hoffman
mhoffman@lightlink.com


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

Re: [lm-sensors] Need some guidance on adhering to the sysfs

[Jean Delvare]

Hi George,

On Thu, 19 Apr 2007 11:13:20 -0600, George T. Joseph wrote:
> > On Thu, 12 Apr 2007 10:50:30 -0600, George T. Joseph wrote:
> > > These chips have 2 temp points.  The low temp (auto_point1_temp) is
> > > simple enough to map to the corresponding register.  The high point
> > > (auto_point2_temp) isn't so easy because it's implemented as a range
> > > with the low temp as a base.  Even worse, range is not an arbitrary
> > > value; it's a value from a list of 16.  So if a user 
> > > chooses 26 for a low point and 50 for a high point, that's a range of
> > > 24 except that 24 isn't a valid range value.  The closest values are
> > > either 20 or 26.67.
> > > Also, if the user sets the high point then changes the low 
> > > point, the high point will probably have to be changed because the new
> > > calculated range might not be a valid value.
> > 
> > Pick the nearest possible value for auto_point2_temp. If
> > auto_point1_temp is changed afterwards, indeed the range register
> > should be adjusted so that auto_point2_temp is preserved as much as
> > possible (principle of least surprise). I understand there 
> > will be some
> > loss, but that's better than ignoring the issue altogether. I do not
> > expect it to be a big deal in practice, as I think the users typically
> > set auto_point1 first.
> 
> OK, I can do that but rather than "nearest" point I'll have to go with
> the lower range value because the higher range value could result in a
> heat related failure.

Well, the user (or user-space application) should never assume that the
value he/she wrote to the sysfs file is exactly what will be applied.
The sysfs file should always be read back afterwards for confirmation.
The value can always be constrained into an arbitrary range or
resolution be trimmed to fit in the register. But I'm fine either way.

> > > Auto_point_pwm is also an issue.  Where there are 4 temp 
> > zones, there
> > > are only 3 pwm outputs and the assignment of pwm to zone 
> > can be changed
> > > by the user or automatically by the chip.   
> > 
> > Automatically, based on what?
> > What is a "temp zone" for this device?
> > 
> > We have a mapping file (auto_channels) between PWM outputs and
> > temperature inputs which is supposed to address this issue.
> 
> The chip has 4 "zones".  Each zone can be configured from 7 sources
> available: 2 remote diodes, the internal chip temp, and 4 PECI bus
> temps.  I've created a temp[1-4]_source file for this.

Let me make sure I understand how the chip works. The chip has 7
temperature inputs, but only 4 can be active at the same time? Or it
has 7 temperature inputs and only 4 can be used for automatic fan speed
control at a given time?

There's only one temperature input in each zone at any given time?

> There are 3 PWM
> outputs, each of which can be controlled by a single zone, or by the
> "hottest of zone 2 or 3",  "hottest of zone 1, 2, or 3", or "hottest of
> zone 1, 2, 3, or 4".  "hottest" is determined in real time by the chip.
> The actual zone controlling a pwm output at any point in time is found
> in a separate register.  I have that exposed as
> pwm[1-3]_auto_zone_assigned.

Am I right assuming that user-space could easily compute this by
comparing the temperature values directly?

> Min and max pwms are associated to the pwm output itself while the temp
> min and maxes are associated to the zones.  Hence the following is valid
> and what I expose...
> pwm[1-3]_auto_channels_temp
> pwm[1-3]_auto_point[1-2]_pwm
> temp[1-4]_auto_point[1-2]_temp
> temp[1-4]_auto_point[1-2]_temp_hyst

This fits OK in the current standard interface, right?

> Another little issue I just realized has to do with what happens to pwm
> output when a temp is BELOW the minimum for the zone.  You can configure
> the pwm output to be either off or run at the minimum set.  If you want
> it to be off, you can't just set the min pwm to 0 because that would
> imply that when the temp does rise to the min temp, the pwm slope would
> start from 0.  That's not good because the fan might not even run until
> the pwm duty cycle is above 50 or 75.

This is a common issue. Most chips include a "spin-up" duty-cycle,
duration pair which is used whenever a PWM output goes from zero to
non-zero in automatic mode. I assume your chip does something like that
(even if it the duty cycle and duration might not be configurable)?

> The behavior needed is that when
> the temp is below the min, the fan is off, when the temp reaches the min
> temp, the fan runs at the min pwm duty cycle then scales up as the temp
> does.  So, I'll have to create a separate sysfs file for this control
> and it's going to need a name.  Can you suggest one that means "turn the
> pwm off if the temp is below the auto_point1_temp"?  
> Maybe pwm[1-3]_auto_point1_pwm_off_if_below :)

In fact, several other chips have a similar feature, but we never
bothered exposing it (i.e. the user has to live with the chip's default
or whatever the BIOS set.) If you really want to expose this, I have no
idea of a nice file name for it, sorry. Name it what you want.

> (...)
> > Feel free to create a file under Documentation/hwmon for your driver
> > and document the limitations if you think the user might be confused.
> 
> It's not difficult, just convoluted.  I have to synthesize sysfs files
> from multiple registers which weren't designed to be related by the
> manufacturer.

You can as well see it the other way around. The manufacturer decided
to split a given information into two separate registers, or to have
one register for two different things. Our standard defines individual
values, one value per file, chips sometimes don't.

> I have a documentation file ready but I'm afraid that rather than
> describing the capabilities of the chip it's going to be mostly
> explaining the fallout from shoehorning it into a shoe that's one size
> to small. :)

Don't blame the standard. Blame the manufacturers for coming up with
different limitations and weird ways of expressing similar ideas with
each new device they design. For example, the fact that only a limited
number of discrete temperature ranges between the two trip points are
allowed is really a limitation of the device. No better standard on our
side would help there. That's exactly the kind of thing I think is
worth documenting. This should save us a number of support mails from
the users.

> > > 3) Adhere to the standard where possible without creating 
> > > the compound relationships.  So what I'd do is...
> > >  temp[1-4]_auto_point_temp_min
> > >  temp[1-4]_auto_point_temp_range
> > >  temp[1-4]_auto_point_temp_hyst
> > >  pwm[1-3]_auto_channels_temp
> > >  pwm[1-3]_auto_point_pwm_min
> > >  pwm[1-3]_auto_point_pwm_max
> > > 
> > > 4) Or I could just name them what they're named in the data sheet...
> > > 
> > >  temp[1-4]_fan_temp_limit
> > >  temp[1-4]_range
> > >  temp[1-4]_hyst
> > >  pwm[1-3]_config
> > >  pwm{1-3]_min
> > >  pwm[1-3]_max
> > 
> > This would be particularly confusing, as these names don't show the
> > relations between the items.
> 
> Which is confusing, 3 or 4?  I agree that 4 might be but 3 is as
> straightforward as you can get.

I meant that proposal 4 was confusing, at least with these short names.
The user can't guess what the files really do without reading the
datasheet or documentation. Proposal 3 is a bit better, but not perfect
either. For example, you know that temp[1-4]_auto_point_temp_range is
the difference between temp[1-4]_auto_point_temp_min (which exists) and
temp[1-4]_auto_point_temp_min (which doesn't exist.) But how does the
user know that? He/she can guess, but maybe he/she won't.

-- 
Jean Delvare

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

Re: [lm-sensors] Need some guidance on adhering to the sysfs

[Juerg Haefliger]

Hi Jean,

On 4/22/07, Jean Delvare <khali@linux-fr.org> wrote:
> Hi Juerg,
>
> On Thu, 19 Apr 2007 12:04:28 -0700, Juerg Haefliger wrote:
> > George, Jean,
> >
> > I'm struggling with the same issue for the DME1737 I'm currently
> > working on. This chip also features temp zones and "hottest of x,y,z"
> > PWM control. The current sysfs standard is not flexible enough to
> > handle these features, especially the combination of a single PWM
> > being controlled by multiple temp inputs and multiple PMWs being
> > controlled by the same temp input. I believe we need another layer of
> > mapping. I.e. temp->pwm is not sufficient, but rather temp->zone->pwm.
> >
> > I therefore propose the add the following sysfs attributes to our standard:
> >
> > zone[1-*]_auto_channels_temp   for temp-to-zone mapping
> > pwm[1-*]_auto_channels_zone   for zone-to-pwm mapping
> > zone[1-*]_auto_point[1-*]_temp   for zone temp auto points.
>
> I don't see what value it adds compared to what we currently have.
>
> We have pwm[1-*]_auto_channels_temp, which is a bit vector. We have one
> file per PWM, one bit per temperature channel, so all in all we have a
> Npwm x Ntemp matrix, or N-N relation between PWM and temperatures. This
> already allows us to handle cases such as "the hottest of tempA and
> tempB control pwmC" or "tempD controls pwmE and pwmF".

Yes, I understand that.


> You propose to add the concept of zone. According to the above, each
> zone could include any temperature channel, so we have a N-N relation
> between zones and temperatures. Then you express another N-N relation
> between PWM channels and zones. As far as I can see, this results in a
> N-N relation between temperatures and PWM, just expressed differently.
> Am I missing something? What do you think it would let us express,
> which the current model doesn't?

What I can't seem to map to our current standard (or maybe I just
don't see it) is the concept of multiple sets of thermal thresholds
for a single temp input. Example: pwm2 is controlled by zone2 and pwm3
is controlled by zone3 but both zone2 and zone3 are controlled by
temp3. Both zone2 & 3 have different thermal thresholds.

With the current standard I can only apply one set of thresholds to
temp3 via temp3_auto_point[1-*]_temp.

...juerg


> --
> Jean Delvare
>

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

Re: [lm-sensors] Need some guidance on adhering to the sysfs

[Juerg Haefliger]

On 4/22/07, Hans de Goede <j.w.r.degoede@hhs.nl> wrote:
> Juerg Haefliger wrote:
> > Hi Jean,
> >
> > On 4/22/07, Jean Delvare <khali@linux-fr.org> wrote:
> >> Hi Juerg,
> >>
> >> On Thu, 19 Apr 2007 12:04:28 -0700, Juerg Haefliger wrote:
> >>> George, Jean,
> >>>
> >>> I'm struggling with the same issue for the DME1737 I'm currently
> >>> working on. This chip also features temp zones and "hottest of x,y,z"
> >>> PWM control. The current sysfs standard is not flexible enough to
> >>> handle these features, especially the combination of a single PWM
> >>> being controlled by multiple temp inputs and multiple PMWs being
> >>> controlled by the same temp input. I believe we need another layer of
> >>> mapping. I.e. temp->pwm is not sufficient, but rather temp->zone->pwm.
> >>>
> >>> I therefore propose the add the following sysfs attributes to our standard:
> >>>
> >>> zone[1-*]_auto_channels_temp   for temp-to-zone mapping
> >>> pwm[1-*]_auto_channels_zone   for zone-to-pwm mapping
> >>> zone[1-*]_auto_point[1-*]_temp   for zone temp auto points.
> >> I don't see what value it adds compared to what we currently have.
> >>
> >> We have pwm[1-*]_auto_channels_temp, which is a bit vector. We have one
> >> file per PWM, one bit per temperature channel, so all in all we have a
> >> Npwm x Ntemp matrix, or N-N relation between PWM and temperatures. This
> >> already allows us to handle cases such as "the hottest of tempA and
> >> tempB control pwmC" or "tempD controls pwmE and pwmF".
> >
> > Yes, I understand that.
> >
> >
> >> You propose to add the concept of zone. According to the above, each
> >> zone could include any temperature channel, so we have a N-N relation
> >> between zones and temperatures. Then you express another N-N relation
> >> between PWM channels and zones. As far as I can see, this results in a
> >> N-N relation between temperatures and PWM, just expressed differently.
> >> Am I missing something? What do you think it would let us express,
> >> which the current model doesn't?
> >
> > What I can't seem to map to our current standard (or maybe I just
> > don't see it) is the concept of multiple sets of thermal thresholds
> > for a single temp input. Example: pwm2 is controlled by zone2 and pwm3
> > is controlled by zone3 but both zone2 and zone3 are controlled by
> > temp3. Both zone2 & 3 have different thermal thresholds.
> >
> > With the current standard I can only apply one set of thresholds to
> > temp3 via temp3_auto_point[1-*]_temp.
> >
>
> Thats easy, AFAIK you can have either temp[1-*]_auto_point[1-*]_temp,
> or pwm[1-*]_auto_point[1-*]_temp, iow you can couple the autopoints
> to either a temp channel or a pwm channel depending on if the
> thresholds are set per temp channel or per pwm channel.

That's not going to work. I can't use temp[1-*]_auto_point[1-*]_temp
because there are multiple sets of thresholds for a single temp input
and I can't use pwm[1-*]_auto_point[1-*]_temp either because then the
"hottest of x,y,z" doesn't work.

...juerg


> I'm not 100% sure though, so lets wait what others have to say too.
>
> Regards,
>
> Hans
>

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

Re: [lm-sensors] Need some guidance on adhering to the sysfs

[Juerg Haefliger]

On 4/22/07, Hans de Goede <j.w.r.degoede@hhs.nl> wrote:
> Juerg Haefliger wrote:
> > On 4/22/07, Hans de Goede <j.w.r.degoede@hhs.nl> wrote:
> >> Juerg Haefliger wrote:
> >>> Hi Jean,
> >>>
> >>> On 4/22/07, Jean Delvare <khali@linux-fr.org> wrote:
> >>>> Hi Juerg,
> >>>>
> >>>> On Thu, 19 Apr 2007 12:04:28 -0700, Juerg Haefliger wrote:
> >>>>> George, Jean,
> >>>>>
> >>>>> I'm struggling with the same issue for the DME1737 I'm currently
> >>>>> working on. This chip also features temp zones and "hottest of x,y,z"
> >>>>> PWM control. The current sysfs standard is not flexible enough to
> >>>>> handle these features, especially the combination of a single PWM
> >>>>> being controlled by multiple temp inputs and multiple PMWs being
> >>>>> controlled by the same temp input. I believe we need another layer of
> >>>>> mapping. I.e. temp->pwm is not sufficient, but rather temp->zone->pwm.
> >>>>>
> >>>>> I therefore propose the add the following sysfs attributes to our standard:
> >>>>>
> >>>>> zone[1-*]_auto_channels_temp   for temp-to-zone mapping
> >>>>> pwm[1-*]_auto_channels_zone   for zone-to-pwm mapping
> >>>>> zone[1-*]_auto_point[1-*]_temp   for zone temp auto points.
> >>>> I don't see what value it adds compared to what we currently have.
> >>>>
> >>>> We have pwm[1-*]_auto_channels_temp, which is a bit vector. We have one
> >>>> file per PWM, one bit per temperature channel, so all in all we have a
> >>>> Npwm x Ntemp matrix, or N-N relation between PWM and temperatures. This
> >>>> already allows us to handle cases such as "the hottest of tempA and
> >>>> tempB control pwmC" or "tempD controls pwmE and pwmF".
> >>> Yes, I understand that.
> >>>
> >>>
> >>>> You propose to add the concept of zone. According to the above, each
> >>>> zone could include any temperature channel, so we have a N-N relation
> >>>> between zones and temperatures. Then you express another N-N relation
> >>>> between PWM channels and zones. As far as I can see, this results in a
> >>>> N-N relation between temperatures and PWM, just expressed differently.
> >>>> Am I missing something? What do you think it would let us express,
> >>>> which the current model doesn't?
> >>> What I can't seem to map to our current standard (or maybe I just
> >>> don't see it) is the concept of multiple sets of thermal thresholds
> >>> for a single temp input. Example: pwm2 is controlled by zone2 and pwm3
> >>> is controlled by zone3 but both zone2 and zone3 are controlled by
> >>> temp3. Both zone2 & 3 have different thermal thresholds.
> >>>
> >>> With the current standard I can only apply one set of thresholds to
> >>> temp3 via temp3_auto_point[1-*]_temp.
> >>>
> >> Thats easy, AFAIK you can have either temp[1-*]_auto_point[1-*]_temp,
> >> or pwm[1-*]_auto_point[1-*]_temp, iow you can couple the autopoints
> >> to either a temp channel or a pwm channel depending on if the
> >> thresholds are set per temp channel or per pwm channel.
> >
> > That's not going to work. I can't use temp[1-*]_auto_point[1-*]_temp
> > because there are multiple sets of thresholds for a single temp input
> > and I can't use pwm[1-*]_auto_point[1-*]_temp either because then the
> > "hottest of x,y,z" doesn't work.
> >
>
> Can't you use pwm[1-*]_auto_channels_temp for that?

Yes, I do. OK, I wasn't clear enough in my previous post. The problem
is the thermal thresholds. How would you implement the following
example with the current sysfs standard?

temp3 maps to zone2
temp3 maps to zone3
zone2 has min and max temp thresholds
zone3 has min and max temp thresholds
pwm2 is controlled by zone2
pwm3 is controlled by hottest of zone2 or zone3

See what the problem is? There are 2 sets of thermal thresholds for
pwm3 and 2 sets for temp3.

...juerg

>
> Regards,
>
> Hans
>

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

Re: [lm-sensors] Need some guidance on adhering to the sysfs

[Juerg Haefliger]

On 4/22/07, Juerg Haefliger <juergh@gmail.com> wrote:
> On 4/22/07, Hans de Goede <j.w.r.degoede@hhs.nl> wrote:
> > Juerg Haefliger wrote:
> > > On 4/22/07, Hans de Goede <j.w.r.degoede@hhs.nl> wrote:
> > >> Juerg Haefliger wrote:
> > >>> Hi Jean,
> > >>>
> > >>> On 4/22/07, Jean Delvare <khali@linux-fr.org> wrote:
> > >>>> Hi Juerg,
> > >>>>
> > >>>> On Thu, 19 Apr 2007 12:04:28 -0700, Juerg Haefliger wrote:
> > >>>>> George, Jean,
> > >>>>>
> > >>>>> I'm struggling with the same issue for the DME1737 I'm currently
> > >>>>> working on. This chip also features temp zones and "hottest of x,y,z"
> > >>>>> PWM control. The current sysfs standard is not flexible enough to
> > >>>>> handle these features, especially the combination of a single PWM
> > >>>>> being controlled by multiple temp inputs and multiple PMWs being
> > >>>>> controlled by the same temp input. I believe we need another layer of
> > >>>>> mapping. I.e. temp->pwm is not sufficient, but rather temp->zone->pwm.
> > >>>>>
> > >>>>> I therefore propose the add the following sysfs attributes to our standard:
> > >>>>>
> > >>>>> zone[1-*]_auto_channels_temp   for temp-to-zone mapping
> > >>>>> pwm[1-*]_auto_channels_zone   for zone-to-pwm mapping
> > >>>>> zone[1-*]_auto_point[1-*]_temp   for zone temp auto points.
> > >>>> I don't see what value it adds compared to what we currently have.
> > >>>>
> > >>>> We have pwm[1-*]_auto_channels_temp, which is a bit vector. We have one
> > >>>> file per PWM, one bit per temperature channel, so all in all we have a
> > >>>> Npwm x Ntemp matrix, or N-N relation between PWM and temperatures. This
> > >>>> already allows us to handle cases such as "the hottest of tempA and
> > >>>> tempB control pwmC" or "tempD controls pwmE and pwmF".
> > >>> Yes, I understand that.
> > >>>
> > >>>
> > >>>> You propose to add the concept of zone. According to the above, each
> > >>>> zone could include any temperature channel, so we have a N-N relation
> > >>>> between zones and temperatures. Then you express another N-N relation
> > >>>> between PWM channels and zones. As far as I can see, this results in a
> > >>>> N-N relation between temperatures and PWM, just expressed differently.
> > >>>> Am I missing something? What do you think it would let us express,
> > >>>> which the current model doesn't?
> > >>> What I can't seem to map to our current standard (or maybe I just
> > >>> don't see it) is the concept of multiple sets of thermal thresholds
> > >>> for a single temp input. Example: pwm2 is controlled by zone2 and pwm3
> > >>> is controlled by zone3 but both zone2 and zone3 are controlled by
> > >>> temp3. Both zone2 & 3 have different thermal thresholds.
> > >>>
> > >>> With the current standard I can only apply one set of thresholds to
> > >>> temp3 via temp3_auto_point[1-*]_temp.
> > >>>
> > >> Thats easy, AFAIK you can have either temp[1-*]_auto_point[1-*]_temp,
> > >> or pwm[1-*]_auto_point[1-*]_temp, iow you can couple the autopoints
> > >> to either a temp channel or a pwm channel depending on if the
> > >> thresholds are set per temp channel or per pwm channel.
> > >
> > > That's not going to work. I can't use temp[1-*]_auto_point[1-*]_temp
> > > because there are multiple sets of thresholds for a single temp input
> > > and I can't use pwm[1-*]_auto_point[1-*]_temp either because then the
> > > "hottest of x,y,z" doesn't work.
> > >
> >
> > Can't you use pwm[1-*]_auto_channels_temp for that?
>
> Yes, I do. OK, I wasn't clear enough in my previous post. The problem
> is the thermal thresholds. How would you implement the following
> example with the current sysfs standard?
>
> temp3 maps to zone2
> temp3 maps to zone3
> zone2 has min and max temp thresholds
> zone3 has min and max temp thresholds
> pwm2 is controlled by zone2
> pwm3 is controlled by hottest of zone2 or zone3
>
> See what the problem is? There are 2 sets of thermal thresholds for
> pwm3 and 2 sets for temp3.

OK just realized that this is a silly example :-) Let's try again:

temp1 maps to zone1
temp3 maps to zone2
temp3 maps to zone3
zone1 has min and max temp thresholds
zone2 has min and max temp thresholds
zone3 has min and max temp thresholds
pwm1 is controlled by zone1
pwm2 is controlled by zone2
pwm3 is controlled by hottest of zone1 or zone3

Not sure if that makes more sense but the DME1737 can certainly be
programmed that way...

> ...juerg
>
> >
> > Regards,
> >
> > Hans
> >
>

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

Re: [lm-sensors] Need some guidance on adhering to the sysfs

[Hans de Goede]

Juerg Haefliger wrote:
> On 4/22/07, Juerg Haefliger <juergh@gmail.com> wrote:
>> On 4/22/07, Hans de Goede <j.w.r.degoede@hhs.nl> wrote:
>> > Juerg Haefliger wrote:
>> > > On 4/22/07, Hans de Goede <j.w.r.degoede@hhs.nl> wrote:
>> > >> Juerg Haefliger wrote:
>> > >>> Hi Jean,
>> > >>>
>> > >>> On 4/22/07, Jean Delvare <khali@linux-fr.org> wrote:
>> > >>>> Hi Juerg,
>> > >>>>
>> > >>>> On Thu, 19 Apr 2007 12:04:28 -0700, Juerg Haefliger wrote:
>> > >>>>> George, Jean,
>> > >>>>>
>> > >>>>> I'm struggling with the same issue for the DME1737 I'm currently
>> > >>>>> working on. This chip also features temp zones and "hottest of 
>> x,y,z"
>> > >>>>> PWM control. The current sysfs standard is not flexible enough to
>> > >>>>> handle these features, especially the combination of a single PWM
>> > >>>>> being controlled by multiple temp inputs and multiple PMWs being
>> > >>>>> controlled by the same temp input. I believe we need another 
>> layer of
>> > >>>>> mapping. I.e. temp->pwm is not sufficient, but rather 
>> temp->zone->pwm.
>> > >>>>>
>> > >>>>> I therefore propose the add the following sysfs attributes to 
>> our standard:
>> > >>>>>
>> > >>>>> zone[1-*]_auto_channels_temp   for temp-to-zone mapping
>> > >>>>> pwm[1-*]_auto_channels_zone   for zone-to-pwm mapping
>> > >>>>> zone[1-*]_auto_point[1-*]_temp   for zone temp auto points.
>> > >>>> I don't see what value it adds compared to what we currently have.
>> > >>>>
>> > >>>> We have pwm[1-*]_auto_channels_temp, which is a bit vector. We 
>> have one
>> > >>>> file per PWM, one bit per temperature channel, so all in all we 
>> have a
>> > >>>> Npwm x Ntemp matrix, or N-N relation between PWM and 
>> temperatures. This
>> > >>>> already allows us to handle cases such as "the hottest of tempA 
>> and
>> > >>>> tempB control pwmC" or "tempD controls pwmE and pwmF".
>> > >>> Yes, I understand that.
>> > >>>
>> > >>>
>> > >>>> You propose to add the concept of zone. According to the above, 
>> each
>> > >>>> zone could include any temperature channel, so we have a N-N 
>> relation
>> > >>>> between zones and temperatures. Then you express another N-N 
>> relation
>> > >>>> between PWM channels and zones. As far as I can see, this 
>> results in a
>> > >>>> N-N relation between temperatures and PWM, just expressed 
>> differently.
>> > >>>> Am I missing something? What do you think it would let us express,
>> > >>>> which the current model doesn't?
>> > >>> What I can't seem to map to our current standard (or maybe I just
>> > >>> don't see it) is the concept of multiple sets of thermal thresholds
>> > >>> for a single temp input. Example: pwm2 is controlled by zone2 
>> and pwm3
>> > >>> is controlled by zone3 but both zone2 and zone3 are controlled by
>> > >>> temp3. Both zone2 & 3 have different thermal thresholds.
>> > >>>
>> > >>> With the current standard I can only apply one set of thresholds to
>> > >>> temp3 via temp3_auto_point[1-*]_temp.
>> > >>>
>> > >> Thats easy, AFAIK you can have either 
>> temp[1-*]_auto_point[1-*]_temp,
>> > >> or pwm[1-*]_auto_point[1-*]_temp, iow you can couple the autopoints
>> > >> to either a temp channel or a pwm channel depending on if the
>> > >> thresholds are set per temp channel or per pwm channel.
>> > >
>> > > That's not going to work. I can't use temp[1-*]_auto_point[1-*]_temp
>> > > because there are multiple sets of thresholds for a single temp input
>> > > and I can't use pwm[1-*]_auto_point[1-*]_temp either because then the
>> > > "hottest of x,y,z" doesn't work.
>> > >
>> >
>> > Can't you use pwm[1-*]_auto_channels_temp for that?
>>
>> Yes, I do. OK, I wasn't clear enough in my previous post. The problem
>> is the thermal thresholds. How would you implement the following
>> example with the current sysfs standard?
>>
>> temp3 maps to zone2
>> temp3 maps to zone3
>> zone2 has min and max temp thresholds
>> zone3 has min and max temp thresholds
>> pwm2 is controlled by zone2
>> pwm3 is controlled by hottest of zone2 or zone3
>>
>> See what the problem is? There are 2 sets of thermal thresholds for
>> pwm3 and 2 sets for temp3.
> 
> OK just realized that this is a silly example :-) Let's try again:
> 
> temp1 maps to zone1
> temp3 maps to zone2
> temp3 maps to zone3
> zone1 has min and max temp thresholds
> zone2 has min and max temp thresholds
> zone3 has min and max temp thresholds
> pwm1 is controlled by zone1
> pwm2 is controlled by zone2
> pwm3 is controlled by hottest of zone1 or zone3
> 
> Not sure if that makes more sense but the DME1737 can certainly be
> programmed that way...
> 

I see your ppoint, and I don't know how to handle that with the current scheme,

Regards,

Hans


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

Re: [lm-sensors] Need some guidance on adhering to the sysfs

[Jean Delvare]

Hi Juerg,

On Sun, 22 Apr 2007 13:58:56 -0700, Juerg Haefliger wrote:
> On 4/22/07, Juerg Haefliger <juergh@gmail.com> wrote:
> > On 4/22/07, Hans de Goede <j.w.r.degoede@hhs.nl> wrote:
> > > Juerg Haefliger wrote:
> > > > That's not going to work. I can't use temp[1-*]_auto_point[1-*]_temp
> > > > because there are multiple sets of thresholds for a single temp input
> > > > and I can't use pwm[1-*]_auto_point[1-*]_temp either because then the
> > > > "hottest of x,y,z" doesn't work.
> > >
> > > Can't you use pwm[1-*]_auto_channels_temp for that?
> >
> > Yes, I do. OK, I wasn't clear enough in my previous post. The problem
> > is the thermal thresholds. How would you implement the following
> > example with the current sysfs standard?
> >
> > temp3 maps to zone2
> > temp3 maps to zone3
> > zone2 has min and max temp thresholds
> > zone3 has min and max temp thresholds
> > pwm2 is controlled by zone2
> > pwm3 is controlled by hottest of zone2 or zone3
> >
> > See what the problem is? There are 2 sets of thermal thresholds for
> > pwm3 and 2 sets for temp3.
> 
> OK just realized that this is a silly example :-) Let's try again:
> 
> temp1 maps to zone1
> temp3 maps to zone2
> temp3 maps to zone3
> zone1 has min and max temp thresholds
> zone2 has min and max temp thresholds
> zone3 has min and max temp thresholds
> pwm1 is controlled by zone1
> pwm2 is controlled by zone2
> pwm3 is controlled by hottest of zone1 or zone3
> 
> Not sure if that makes more sense but the DME1737 can certainly be
> programmed that way...

If the limits are zone attributes and all sorts of mapping are
possible, then indeed our current interface doesn't support that.

-- 
Jean Delvare

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

Re: [lm-sensors] Need some guidance on adhering to the sysfs

[Jean Delvare]

Hi Mark,

On Sun, 22 Apr 2007 13:12:07 -0400, Mark M. Hoffman wrote:
> > On Thu, 19 Apr 2007 12:04:28 -0700, Juerg Haefliger wrote:
> > > I'm struggling with the same issue for the DME1737 I'm currently
> > > working on. This chip also features temp zones and "hottest of x,y,z"
> > > PWM control. The current sysfs standard is not flexible enough to
> > > handle these features, especially the combination of a single PWM
> > > being controlled by multiple temp inputs and multiple PMWs being
> > > controlled by the same temp input. I believe we need another layer of
> > > mapping. I.e. temp->pwm is not sufficient, but rather temp->zone->pwm.
> > > 
> > > I therefore propose the add the following sysfs attributes to our standard:
> > > 
> > > zone[1-*]_auto_channels_temp   for temp-to-zone mapping
> > > pwm[1-*]_auto_channels_zone   for zone-to-pwm mapping
> > > zone[1-*]_auto_point[1-*]_temp   for zone temp auto points.
> 
> * Jean Delvare <khali@linux-fr.org> [2007-04-22 17:29:04 +0200]:
> > I don't see what value it adds compared to what we currently have.
> > 
> > We have pwm[1-*]_auto_channels_temp, which is a bit vector. We have one
> > file per PWM, one bit per temperature channel, so all in all we have a
> > Npwm x Ntemp matrix, or N-N relation between PWM and temperatures. This
> > already allows us to handle cases such as "the hottest of tempA and
> > tempB control pwmC" or "tempD controls pwmE and pwmF".
> > 
> > You propose to add the concept of zone. According to the above, each
> > zone could include any temperature channel, so we have a N-N relation
> > between zones and temperatures. Then you express another N-N relation
> > between PWM channels and zones. As far as I can see, this results in a
> > N-N relation between temperatures and PWM, just expressed differently.
> > Am I missing something? What do you think it would let us express,
> > which the current model doesn't?
> 
> I was never a big fan of the pwm_auto standard interface in the first place.
> IMHO, the various chips implement automatic fan control in so many different
> ways that the abstraction is bound to be mis-aligned for most chips, no matter
> how we model it.
> 
> Or if not, then the abstraction would necessarily have so many options that it
> hardly even counts as an abstraction.

Oh well, you're right. It is quite obvious now that our (my?)
standardization attempt of the automatic fan speed control interface is
a complete failure. Chips are just too different. We would need an
industry standard for the manufacturers to follow. Trying to establish
a standard at the software level after the fact was silly. Sorry, I
guess I should have admitted sooner.

> I don't see it in Documentation/hwmon/sysfs-interface, but IIRC it was agreed
> that the pwm_auto standard interface was optional.  So, a driver may support
> that interface as best it can, OR it can support one which describes its actual
> capabilities precisely (so long as there is no collision of sysfs names).  It
> is important to note that libsensors has no pwm_auto support anyway, and none
> is planned.

We do need a standard for _manual_ fan speed control, including PWM
frequency and output mode (DC vs. PWM), and that one is already in
place and works. Not for libsensors, but for pwmconfig/fancontrol.

I wouldn't go as far as saying that we wouldn't need a standard for
automatic fan speed control. If we had a working one, I guess someone
could write a nice graphical interface to tweak the parameters. But the
fact is that the chips are too different, so we wouldn't get around
device-specific instructions anyway.

> So I agree with Jean that we shouldn't extend the "standard" interface.  If
> you feel that the standard interface is insufficient for your hardware, then
> go ahead and create something completely different.  If the chip is "close"
> to fitting the standard interface, I would prefer you support that as best
> you can.

If we're going to admit that we can't standardize the automatic fan
speed control interface, then I'd rather not advertise a particular
model in Documentation/hwmon/sysfs-interface at all. Let's just rip it
off altogether, and let driver authors implement things the way they
like. The only thing that matters then is that names make sense, that
standard units are used, and that they stick to the one value per file
rule.

I also think that we still want to enforce the rule that we only expose
absolute values and not ranges.

-- 
Jean Delvare

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

Re: [lm-sensors] Need some guidance on adhering to the sysfs

[Juerg Haefliger]

On 4/24/07, Jean Delvare <khali@linux-fr.org> wrote:
> Hi Mark,
>
> On Sun, 22 Apr 2007 13:12:07 -0400, Mark M. Hoffman wrote:
> > > On Thu, 19 Apr 2007 12:04:28 -0700, Juerg Haefliger wrote:
> > > > I'm struggling with the same issue for the DME1737 I'm currently
> > > > working on. This chip also features temp zones and "hottest of x,y,z"
> > > > PWM control. The current sysfs standard is not flexible enough to
> > > > handle these features, especially the combination of a single PWM
> > > > being controlled by multiple temp inputs and multiple PMWs being
> > > > controlled by the same temp input. I believe we need another layer of
> > > > mapping. I.e. temp->pwm is not sufficient, but rather temp->zone->pwm.
> > > >
> > > > I therefore propose the add the following sysfs attributes to our standard:
> > > >
> > > > zone[1-*]_auto_channels_temp   for temp-to-zone mapping
> > > > pwm[1-*]_auto_channels_zone   for zone-to-pwm mapping
> > > > zone[1-*]_auto_point[1-*]_temp   for zone temp auto points.
> >
> > * Jean Delvare <khali@linux-fr.org> [2007-04-22 17:29:04 +0200]:
> > > I don't see what value it adds compared to what we currently have.
> > >
> > > We have pwm[1-*]_auto_channels_temp, which is a bit vector. We have one
> > > file per PWM, one bit per temperature channel, so all in all we have a
> > > Npwm x Ntemp matrix, or N-N relation between PWM and temperatures. This
> > > already allows us to handle cases such as "the hottest of tempA and
> > > tempB control pwmC" or "tempD controls pwmE and pwmF".
> > >
> > > You propose to add the concept of zone. According to the above, each
> > > zone could include any temperature channel, so we have a N-N relation
> > > between zones and temperatures. Then you express another N-N relation
> > > between PWM channels and zones. As far as I can see, this results in a
> > > N-N relation between temperatures and PWM, just expressed differently.
> > > Am I missing something? What do you think it would let us express,
> > > which the current model doesn't?
> >
> > I was never a big fan of the pwm_auto standard interface in the first place.
> > IMHO, the various chips implement automatic fan control in so many different
> > ways that the abstraction is bound to be mis-aligned for most chips, no matter
> > how we model it.
> >
> > Or if not, then the abstraction would necessarily have so many options that it
> > hardly even counts as an abstraction.
>
> Oh well, you're right. It is quite obvious now that our (my?)
> standardization attempt of the automatic fan speed control interface is
> a complete failure. Chips are just too different. We would need an
> industry standard for the manufacturers to follow. Trying to establish
> a standard at the software level after the fact was silly. Sorry, I
> guess I should have admitted sooner.
>
> > I don't see it in Documentation/hwmon/sysfs-interface, but IIRC it was agreed
> > that the pwm_auto standard interface was optional.  So, a driver may support
> > that interface as best it can, OR it can support one which describes its actual
> > capabilities precisely (so long as there is no collision of sysfs names).  It
> > is important to note that libsensors has no pwm_auto support anyway, and none
> > is planned.
>
> We do need a standard for _manual_ fan speed control, including PWM
> frequency and output mode (DC vs. PWM), and that one is already in
> place and works. Not for libsensors, but for pwmconfig/fancontrol.
>
> I wouldn't go as far as saying that we wouldn't need a standard for
> automatic fan speed control. If we had a working one, I guess someone
> could write a nice graphical interface to tweak the parameters. But the
> fact is that the chips are too different, so we wouldn't get around
> device-specific instructions anyway.
>
> > So I agree with Jean that we shouldn't extend the "standard" interface.  If
> > you feel that the standard interface is insufficient for your hardware, then
> > go ahead and create something completely different.  If the chip is "close"
> > to fitting the standard interface, I would prefer you support that as best
> > you can.
>
> If we're going to admit that we can't standardize the automatic fan
> speed control interface, then I'd rather not advertise a particular
> model in Documentation/hwmon/sysfs-interface at all. Let's just rip it
> off altogether, and let driver authors implement things the way they
> like. The only thing that matters then is that names make sense, that
> standard units are used, and that they stick to the one value per file
> rule.

I wouldn't go that far. I think the current model is pretty decent and
if we add the concept of zones it becomes more flexible and should
cover most of the chips (granted I don't know that many so that's just
an assumption). Even if we can only cover lets say 80% of the chips I
still think it's worthwhile to have a standard for those. Otherwise
the implementations will be all over the place.

...juerg


> I also think that we still want to enforce the rule that we only expose
> absolute values and not ranges.
>
> --
> Jean Delvare
>

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