On Sun, Aug 06, 2006 at 03:37:22PM +0000, Juan Romero Pardines wrote: > > Module Name: src > Committed By: xtraeme > Date: Sun Aug 6 15:37:22 UTC 2006 > > 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 > > Log Message: > AMD PowerNow!/Cool`n'Quiet driver for NetBSD/amd64, > adapted from OpenBSD. > > Tested on a few machines: > > http://bigbird.dohd.org:3021/NetBSD/dmesg > http://www.bsd.org.il/netbsd/acpi/dmesg > > 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. -- Quentin Garnier - cube%cubidou.net@localhost - cube%NetBSD.org@localhost "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.
Attachment:
pgpxF2_lcCCGW.pgp
Description: PGP signature