Subject: Re: CVS commit: src
To: Juan Romero Pardines <xtraeme@netbsd.org>
From: Quentin Garnier <cube@cubidou.net>
List: source-changes
Date: 08/06/2006 19:38:41
--x0PBmTWHB3bjoOVh
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sun, Aug 06, 2006 at 03:37:22PM +0000, Juan Romero Pardines wrote:
>=20
> Module Name:	src
> Committed By:	xtraeme
> Date:		Sun Aug  6 15:37:22 UTC 2006
>=20
> Modified Files:
> 	src/doc: CHANGES
> 	src/share/man/man4: options.4
> 	src/sys/arch/amd64/amd64: identcpu.c
> 	src/sys/arch/amd64/conf: GENERIC files.amd64
> 	src/sys/arch/amd64/include: cpu.h
> 	src/sys/arch/x86/conf: files.x86
> Added Files:
> 	src/sys/arch/amd64/amd64: powernow_k8.c
> 	src/sys/arch/x86/include: powernow.h
> 	src/sys/arch/x86/x86: powernow_common.c
>=20
> Log Message:
> AMD PowerNow!/Cool`n'Quiet driver for NetBSD/amd64,
> adapted from OpenBSD.
>=20
> Tested on a few machines:
>=20
> http://bigbird.dohd.org:3021/NetBSD/dmesg
> http://www.bsd.org.il/netbsd/acpi/dmesg
>=20
> Thanks to cube, elad and others for testing and fixes.

Hmm, why do I get thanked?  Sure I made a few comments after a quick
glance at the code but you ignored them apparently.

I expect this to break on EM64T, but I told you that already...

Also, why didn't you try adding support for that on i386, too?  I
haven't seen anything in the spec that restricts Cool'n'Quiet to long
mode.  And I still feel bitmask_snprintf would be more suitable than
your own equivalent.

Some other comments:

 - k8_powernow_setperf could return meaningful errors (e.g., EBUSY in
   the PN8_STA_PENDING(status) case), which would then be returned by
   the sysctl handler

 - you don't free freq_names if k8pnow_current_state is NULL (what if
   the PSB doesn't list the current state?  you know vendors...  I
   certainly expect one to ship a laptop with a broken PSB but a correct
   ACPI table)

 - the spec indicates that FID and VID changes of a dual-core processor
   must me co-ordinated.

Glad I could help.  Please have your code reviewed on tech-kern before
you commit.

--=20
Quentin Garnier - cube@cubidou.net - cube@NetBSD.org
"When I find the controls, I'll go where I like, I'll know where I want
to be, but maybe for now I'll stay right here on a silent sea."
KT Tunstall, Silent Sea, Eye to the Telescope, 2004.

--x0PBmTWHB3bjoOVh
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (NetBSD)

iQEVAwUBRNYpINgoQloHrPnoAQLuygf/bh+3++4K7EjNZEah5l+f740zDqzO+oKt
0iz/XJCjMiFKgMSKY9l2wzKZn19ElaZ9doRWNVZALDw8nAwtxZL3us+WDne/WsOe
7EVTldPU6fWH9jQzWYRuG6mQ5TVmgezd4OQn5K/f7THWu5kW7yNL64rCQ1Pv6Ww/
7CFHptlWUX/fHQxzIMoRBzfEKySgufQf3KPxarehcDl9V2t9ZfsDYn+7iBSoupLd
vfpUGGbV9D7zbseWQASykSALYkU5zlSjoCMUTOaXqaerWjUCGwoEx1vF6btS49gF
KVrKifaTIe42VjUGEU/VUQKmu4RoNxHLmHY2fT+9o0Utx0Eb+dXfAA==
=4DFL
-----END PGP SIGNATURE-----

--x0PBmTWHB3bjoOVh--