> Index: dev/dfs.c

> +static int voltage;

There is no reason for this variable to be global. You should put it in
your device' softc instead.

> +int
> +dfs_match(struct device *parent, void *arg, void *aux)
> +{
> +     struct confargs *ca = aux;
> +     uint16_t cpu;
> +
> +     if (strcmp(ca->ca_name, "cpu-vcore-select") != 0)
> +             return (0);
> +
> +     cpu = ppc_mfpvr() >> 16;

Is there any possibility for this device to attach to multiprocessor
machines? If so, what if there are multiple cpu-vcore-select nodes, one
per processor? Oh well, I suppose there won't be multiprocessor systems
with different cpu models... at least not on macppc.

> +     cpu = ppc_mfpvr() >> 16;
> +     if (cpu == PPC_CPU_MPC7448)
> +             printf(", %d MHz\n", ppc_maxfreq / 4);
> +     else
> +             printf(" MHz\n");

Nitpicking, but I'd rather see the optional /4 freq be printed as ", %d"
and an unconditional " MHz\n" in all cases.

> +void
> +dfs_setperf(int perflevel)
> +{
> +     if (perflevel > 50) {
> +             if (ppc_curfreq != ppc_maxfreq) {
> +                     macobio_write(voltage, GPIO_DDR_OUTPUT | 1);
> +                     DELAY(1000);

Speaking of DELAY()... it is implemented using the processor internal
counter register. Is this register impacted by frequency changes? If so,
shouldn't you update the computed ns_per_tick delay() constant?

Also, this register is used to feed the timecounter. Is the clock still
accurate after running for a while at a lower frequency? Is there any
other, fixed, clock source which can be used as a safe monotonic clock
source?

Miod

Reply via email to