Re: smu(4) PWM Fan Support

2016-05-12 Thread Marcus Glocker
On Thu, May 12, 2016 at 08:39:01PM +, Miod Vallat wrote:

> > > fcu(4) could benefit from a similar change, as there may be pwm fans
> > > there too. At least RackMac3,1 has two pwm fans according to the eeprom
> > > -p output.
> > 
> > Hmm, looking at the fcu(4) code it already should support pwm fans, no?
> 
> Doh! I was looking for `fan' in the sensors output and did not notice
> `percent'. Sorry for the noise.

:-)

That's exactly the reason why the first diff uses the rpm value in
favour of the pwm percentage value.  It's more intuitive to expect
all fans to show up as a fan sensor type instead of a percentage
type.  But I guess since other drivers also use the percentage value
for pwm fans it's more strict to keep this, hence the second diff.



Re: smu(4) PWM Fan Support

2016-05-12 Thread Miod Vallat
> > fcu(4) could benefit from a similar change, as there may be pwm fans
> > there too. At least RackMac3,1 has two pwm fans according to the eeprom
> > -p output.
> 
> Hmm, looking at the fcu(4) code it already should support pwm fans, no?

Doh! I was looking for `fan' in the sensors output and did not notice
`percent'. Sorry for the noise.



Re: smu(4) PWM Fan Support

2016-05-12 Thread Marcus Glocker
On Tue, May 10, 2016 at 11:46:24PM +0200, Marcus Glocker wrote:

> I've recently noticed that two of five fans in my G5 don't spin up.
> That's because smu(4) currently just supports RPM fans.
> The attached diff adds initial support for PWM fans as well.
> 
> sysctl before:
> # sysctl -a | grep fan
> hw.sensors.smu0.fan0=994 RPM (Rear Fan 0)
> hw.sensors.smu0.fan1=994 RPM (Rear fan 1)
> hw.sensors.smu0.fan2=994 RPM (Front Fan)
> 
> sysctl after:
> # sysctl -a | grep fan
> hw.sensors.smu0.fan0=994 RPM (Rear Fan 0)
> hw.sensors.smu0.fan1=994 RPM (Rear fan 1)
> hw.sensors.smu0.fan2=994 RPM (Front Fan)
> hw.sensors.smu0.fan3=589 RPM (Slots Fan)
> hw.sensors.smu0.fan4=589 RPM (Drive Bay)
> 
> I was first thinking to introduce a new sensor type in sys/sensor.h
> called SENSOR_FANPWM, but finally I think it's more intuitive to
> display the RPM value for all fans in general.

Alternatively here a diff which displays the fan pwm instead of the rpm.
Maybe this is more straight forward ...
 
> In case this makes it in I would like to split the RPM read which
> is currently done in smu_fan_refresh() in a own function as next,
> same as for the PWM read.  Also with the background that there seems
> to be another set/read method for new style fans which may fail
> currently which we could implement.


Index: sys/arch/macppc/dev/smu.c
===
RCS file: /cvs/src/sys/arch/macppc/dev/smu.c,v
retrieving revision 1.28
diff -u -p -u -p -r1.28 smu.c
--- sys/arch/macppc/dev/smu.c   4 May 2016 08:20:58 -   1.28
+++ sys/arch/macppc/dev/smu.c   12 May 2016 20:09:04 -
@@ -44,6 +44,9 @@ struct smu_fan {
u_int16_t   min_rpm;
u_int16_t   max_rpm;
u_int16_t   unmanaged_rpm;
+   u_int16_t   min_pwm;
+   u_int16_t   max_pwm;
+   u_int16_t   unmanaged_pwm;
struct ksensor  sensor;
 };
 
@@ -144,6 +147,9 @@ int smu_time_read(time_t *);
 intsmu_time_write(time_t);
 intsmu_get_datablock(struct smu_softc *sc, u_int8_t, u_int8_t *, size_t);
 intsmu_fan_set_rpm(struct smu_softc *, struct smu_fan *, u_int16_t);
+intsmu_fan_set_pwm(struct smu_softc *, struct smu_fan *, u_int16_t);
+intsmu_fan_read_pwm(struct smu_softc *, struct smu_fan *, u_int16_t *,
+   u_int16_t *);
 intsmu_fan_refresh(struct smu_softc *, struct smu_fan *);
 intsmu_sensor_refresh(struct smu_softc *, struct smu_sensor *);
 void   smu_refresh_sensors(void *);
@@ -250,7 +256,7 @@ smu_attach(struct device *parent, struct
time_read = smu_time_read;
time_write = smu_time_write;
 
-   /* Fans */
+   /* RPM Fans */
node = OF_getnodebyname(ca->ca_node, "rpm-fans");
if (node == 0)
node = OF_getnodebyname(ca->ca_node, "fans");
@@ -260,7 +266,7 @@ smu_attach(struct device *parent, struct
continue;
 
if (strcmp(type, "fan-rpm-control") != 0) {
-   printf(": unsupported fan type: %s\n", type);
+   printf(": unsupported rpm-fan type: %s\n", type);
return;
}
 
@@ -299,6 +305,53 @@ smu_attach(struct device *parent, struct
 #endif
}
 
+   /* PWM Fans */
+   node = OF_getnodebyname(ca->ca_node, "pwm-fans");
+   for (node = OF_child(node); node; node = OF_peer(node)) {
+   if (OF_getprop(node, "reg", , sizeof reg) <= 0 ||
+   OF_getprop(node, "device_type", type, sizeof type) <= 0)
+   continue;
+
+   if (strcmp(type, "fan-pwm-control") != 0) {
+   printf(": unsupported pwm-fan type: %s\n", type);
+   return;
+   }
+
+   if (sc->sc_num_fans >= SMU_MAXFANS) {
+   printf(": too many fans\n");
+   return;
+   }
+
+   fan = >sc_fans[sc->sc_num_fans++];
+   fan->sensor.type = SENSOR_PERCENT;
+   fan->sensor.flags = SENSOR_FINVALID;
+   fan->reg = reg;
+
+   if (OF_getprop(node, "min-value", , sizeof val) <= 0)
+   val = 0;
+   fan->min_pwm = val;
+   if (OF_getprop(node, "max-value", , sizeof val) <= 0)
+   val = 0x;
+   fan->max_pwm = val;
+   if (OF_getprop(node, "unmanage-value", , sizeof val) > 0)
+   fan->unmanaged_pwm = val;
+   else if (OF_getprop(node, "safe-value", , sizeof val) > 0)
+   fan->unmanaged_pwm = val;
+   else
+   fan->unmanaged_pwm = fan->min_pwm;
+
+   if (OF_getprop(node, "location", loc, sizeof loc) <= 0)
+   strlcpy(loc, "Unknown", sizeof loc);
+   strlcpy(fan->sensor.desc, loc, sizeof sensor->sensor.desc);
+
+   /* Start running fans at their 

Re: smu(4) PWM Fan Support

2016-05-12 Thread Marcus Glocker
On Wed, May 11, 2016 at 07:53:56PM +, Miod Vallat wrote:

> 
> > I've recently noticed that two of five fans in my G5 don't spin up.
> > That's because smu(4) currently just supports RPM fans.
> > The attached diff adds initial support for PWM fans as well.
> 
> fcu(4) could benefit from a similar change, as there may be pwm fans
> there too. At least RackMac3,1 has two pwm fans according to the eeprom
> -p output.

Hmm, looking at the fcu(4) code it already should support pwm fans, no?



Re: smu(4) PWM Fan Support

2016-05-11 Thread Miod Vallat

> I've recently noticed that two of five fans in my G5 don't spin up.
> That's because smu(4) currently just supports RPM fans.
> The attached diff adds initial support for PWM fans as well.

fcu(4) could benefit from a similar change, as there may be pwm fans
there too. At least RackMac3,1 has two pwm fans according to the eeprom
-p output.



smu(4) PWM Fan Support

2016-05-10 Thread Marcus Glocker
I've recently noticed that two of five fans in my G5 don't spin up.
That's because smu(4) currently just supports RPM fans.
The attached diff adds initial support for PWM fans as well.

sysctl before:
# sysctl -a | grep fan
hw.sensors.smu0.fan0=994 RPM (Rear Fan 0)
hw.sensors.smu0.fan1=994 RPM (Rear fan 1)
hw.sensors.smu0.fan2=994 RPM (Front Fan)

sysctl after:
# sysctl -a | grep fan
hw.sensors.smu0.fan0=994 RPM (Rear Fan 0)
hw.sensors.smu0.fan1=994 RPM (Rear fan 1)
hw.sensors.smu0.fan2=994 RPM (Front Fan)
hw.sensors.smu0.fan3=589 RPM (Slots Fan)
hw.sensors.smu0.fan4=589 RPM (Drive Bay)

I was first thinking to introduce a new sensor type in sys/sensor.h
called SENSOR_FANPWM, but finally I think it's more intuitive to
display the RPM value for all fans in general.

In case this makes it in I would like to split the RPM read which
is currently done in smu_fan_refresh() in a own function as next,
same as for the PWM read.  Also with the background that there seems
to be another set/read method for new style fans which may fail
currently which we could implement.


Index: sys/arch/macppc/dev/smu.c
===
RCS file: /cvs/src/sys/arch/macppc/dev/smu.c,v
retrieving revision 1.28
diff -u -p -u -p -r1.28 smu.c
--- sys/arch/macppc/dev/smu.c   4 May 2016 08:20:58 -   1.28
+++ sys/arch/macppc/dev/smu.c   10 May 2016 21:26:39 -
@@ -41,6 +41,13 @@ voidsmu_attach(struct device *, stru
 
 struct smu_fan {
u_int8_treg;
+   enum {
+   SMU_SENSOR_FANRPM,
+   SMU_SENSOR_FANPWM
+   } type;
+   u_int16_t   min_pwm;
+   u_int16_t   max_pwm;
+   u_int16_t   unmanaged_pwm;
u_int16_t   min_rpm;
u_int16_t   max_rpm;
u_int16_t   unmanaged_rpm;
@@ -144,6 +151,9 @@ int smu_time_read(time_t *);
 intsmu_time_write(time_t);
 intsmu_get_datablock(struct smu_softc *sc, u_int8_t, u_int8_t *, size_t);
 intsmu_fan_set_rpm(struct smu_softc *, struct smu_fan *, u_int16_t);
+intsmu_fan_set_pwm(struct smu_softc *, struct smu_fan *, u_int16_t);
+intsmu_fan_read_pwm(struct smu_softc *, struct smu_fan *, u_int16_t *,
+   u_int16_t *);
 intsmu_fan_refresh(struct smu_softc *, struct smu_fan *);
 intsmu_sensor_refresh(struct smu_softc *, struct smu_sensor *);
 void   smu_refresh_sensors(void *);
@@ -250,7 +260,7 @@ smu_attach(struct device *parent, struct
time_read = smu_time_read;
time_write = smu_time_write;
 
-   /* Fans */
+   /* RPM Fans */
node = OF_getnodebyname(ca->ca_node, "rpm-fans");
if (node == 0)
node = OF_getnodebyname(ca->ca_node, "fans");
@@ -260,7 +270,7 @@ smu_attach(struct device *parent, struct
continue;
 
if (strcmp(type, "fan-rpm-control") != 0) {
-   printf(": unsupported fan type: %s\n", type);
+   printf(": unsupported rpm-fan type: %s\n", type);
return;
}
 
@@ -273,6 +283,7 @@ smu_attach(struct device *parent, struct
fan->sensor.type = SENSOR_FANRPM;
fan->sensor.flags = SENSOR_FINVALID;
fan->reg = reg;
+   fan->type = SMU_SENSOR_FANRPM;
 
if (OF_getprop(node, "min-value", , sizeof val) <= 0)
val = 0;
@@ -299,6 +310,54 @@ smu_attach(struct device *parent, struct
 #endif
}
 
+   /* PWM Fans */
+   node = OF_getnodebyname(ca->ca_node, "pwm-fans");
+   for (node = OF_child(node); node; node = OF_peer(node)) {
+   if (OF_getprop(node, "reg", , sizeof reg) <= 0 ||
+   OF_getprop(node, "device_type", type, sizeof type) <= 0)
+   continue;
+
+   if (strcmp(type, "fan-pwm-control") != 0) {
+   printf(": unsupported pwm-fan type: %s\n", type);
+   return;
+   }
+
+   if (sc->sc_num_fans >= SMU_MAXFANS) {
+   printf(": too many fans\n");
+   return;
+   }
+
+   fan = >sc_fans[sc->sc_num_fans++];
+   fan->sensor.type = SENSOR_FANRPM;
+   fan->sensor.flags = SENSOR_FINVALID;
+   fan->reg = reg;
+   fan->type = SMU_SENSOR_FANPWM;
+
+   if (OF_getprop(node, "min-value", , sizeof val) <= 0)
+   val = 0;
+   fan->min_pwm = val;
+   if (OF_getprop(node, "max-value", , sizeof val) <= 0)
+   val = 0x;
+   fan->max_pwm = val;
+   if (OF_getprop(node, "unmanage-value", , sizeof val) > 0)
+   fan->unmanaged_pwm = val;
+   else if (OF_getprop(node, "safe-value", , sizeof val) > 0)
+   fan->unmanaged_pwm = val;
+   else
+