> Date: Tue, 26 Apr 2011 18:29:16 +0530
> From: Martin Pieuchot <[email protected]>
> 
> The following diff adds support for dfs. It requires my precedent patch
> about GPIOs. I don't have a machine with a MPC7448 so it's only tested
> with a MPC7447A.
> 
> I'm also interested in various device-tree dumps for further development.
> If you can send me yours, contact me off list.
> 
> Comments ?

Looks good!  Unfortunately the G4 mini (which has the MPC7447A)
doesn't support this.

Just two comments:

> Index: dev/dfs.c
> ===================================================================
> RCS file: dev/dfs.c
> diff -N dev/dfs.c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ dev/dfs.c 26 Apr 2011 12:13:22 -0000
> @@ -0,0 +1,150 @@
> +/*   $OpenBSD$       */
> +/*
> + * Copyright (c) 2011 Martin Pieuchot <[email protected]>
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include <sys/param.h>
> +#include <sys/filedesc.h>

Why do you need to include <sys/filedesc.h> for?

> +#include <sys/sysctl.h>
> +
> +#include <machine/cpu.h>
> +#include <machine/autoconf.h>
> +#include <macppc/pci/macobio.h>
> +
> +#define      DFS2    (1 << 22)       /* Divide-by-Two */
> +#define      DFS4    (1 << 23)       /* Divide-by-Four (MPC7448 Specific) */
> +
> +extern int perflevel;
> +static int voltage;

You should store the voltage inside a softc (pointed out by miod@).

> +int  dfs_match(struct device *, void *, void *);
> +void dfs_attach(struct device *, struct device *, void *);
> +void dfs_setperf(int);
> +void dfs_scale_frequency(u_int);
> +
> +struct cfattach dfs_ca = {
> +     sizeof(struct device), dfs_match, dfs_attach
> +};
> +
> +struct cfdriver dfs_cd = {
> +     NULL, "dfs", DV_DULL
> +};
> +
> +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;
> +     if (cpu == PPC_CPU_MPC7447A || cpu == PPC_CPU_MPC7448)
> +                     return (1);
> +
> +     return (0);
> +}
> +
> +void
> +dfs_attach(struct device *parent, struct device *self, void *aux)
> +{
> +     struct confargs *ca = aux;
> +     uint32_t hid1;
> +     uint16_t cpu;
> +
> +     voltage = ca->ca_reg[0];
> +
> +     hid1 = ppc_mfhid1();
> +
> +     if (hid1 & DFS4) {
> +             ppc_curfreq = ppc_maxfreq / 4;
> +             perflevel = 25;
> +     } else if (hid1 & DFS2) {
> +             ppc_curfreq = ppc_maxfreq / 2;
> +             perflevel = 50;
> +     }
> +
> +     cpu_setperf = dfs_setperf;
> +
> +     printf(": Dynamic Frequency Switching, speeds: ");
> +     printf("%d, %d", ppc_maxfreq, ppc_maxfreq / 2);

We typically use pure lower case for stuff that shows up in dmesg.
And I'm not sure the "Dynamic Frequency Switching" part really adds
useful information.  Perhaps just change this in:

> +     printf(": speeds: %d, %d", ppc_maxfreq, ppc_maxfreq / 2);

Reply via email to