On Tue, 12 Sep 2006 22:42:00 +0200
Quentin Garnier <cube@cubidou.net> wrote:

> The thing that your are missing in that part is that the
> "smcpufreq_freqlist" and "smcpufreq_currfreq" callbacks are not
> needed: the backend driver will set its own properties and all you
> have to do in sysmon_cpufreq is to keep references to those
> properties;  that way, the dictionary you send to userland will be
> built naturally.

Do you mean that I should use proplib on the MD driver? I wanted to
avoid that...
 
> "smcpufreq_snewfreq" can be kept just for now;  once it's time to deal
> with SMP (it is not time), we'll see how that should be done.

Ok.

> I suggested the name "powerctl" as something that would support much
> more than that in the future.  I see it as something very much like
> wsconsctl(8), where the command-line switches select the type of the
> considered object, and it then displays its properties.  powerctl(8)
> could be the entrypoint for CPU frequency control, but also things
> like thermal zone management and sleep state control.  It could also
> provide means to set policies so that powerd(8) would work magics to
> make the machine perform just as the user wishes.  This is long term,
> though.

I agree completely, I was just playing with sysmon_cpufreq... so it's
still simple.
 
> [...]
> > [juan@nocturno][~]> ./sysmon_cpufreq/powerctl/powerctl 
> > cpu0
> >         current:        800 MHz [998 mV]
> >         frequencies:    800 1200 1400 1600 (in MHz)
> >         voltages:       998 1004 1120 1190 (in mV)
> 
> I honestly don't see the point of ever exposing voltage to the user.
> That's purely an aspect internal to the CPU driver;  some technologies
> other than SpeedStep and Cool'n'Quiet might expose different pieces of
> information.

I thought the same hours ago... I will remove this part.
 
> The code and the design.
> 
> A quick note:
> 
> +	for (smcf = LIST_FIRST(&sysmon_cpufreq_list); smcf != NULL;
> +	     smcf = LIST_NEXT(smcf, smcf_list)) {
> +		if (strcmp(smcf->smcf_name, name) == 0)
> +			break;
> +	}
> 
> You know, that's exactly what LIST_FOREACH is all about...

Ok, I'll play with that :-)

> It's a good start, but there's still a lot to do.

Thanks for your comments Quentin :-)