Date: Mon, 19 Oct 2009 12:17:22 +0100 From: Rui Paulo <rpaulo@freebsd.org> To: Andriy Gapon <avg@icyb.net.ua> Cc: freebsd-hackers@freebsd.org, freebsd-acpi@freebsd.org Subject: Re: SB7xx watchdog: new driver for review and testing Message-ID: <5E0DD277-CAA3-4F01-8561-35CF6C511718@freebsd.org> In-Reply-To: <4ADB764C.2010900@icyb.net.ua> References: <4ADB764C.2010900@icyb.net.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On 18 Oct 2009, at 21:10, Andriy Gapon wrote: > Please review and/or test a new driver for watchdog driver included > into AMD SB7xx: > http://people.freebsd.org/~avg/amdsbwd.tgz > I have tested this driver only with SB700 on Gigabyte GA-MA780G-UD3H > motherboard. > > ichwd driver was used as a starting point for this driver. This can > be seen from > some function names, general code organization and some small code > snippets. > Many thanks to ichwd authors and maintainers! > > Right now I have infrastructure only for building this driver as a > module. > > Things for which that I need the most feedback/ideas: > 1. If the driver actually works on your hardware and the hardware > description. > The driver can be tested by loading the driver and doing 'watchdog - > t <small > number>'. Having debug.bootverbose=1 may provide additional useful > info. > And better to test this from single-user mode with filesystems > mounted r/o. > > 2. Better name for the driver. amdsbwd stands for "AMD S(outh)B(ridge) > WatchDog", but this abbreviation could be cryptic to decipher. > > 3. Proper location for this driver. > At least on my system this driver needs resources (I/O ports and MEM > range) that > are claimed by ACPI, thus I've made it a child of acpi bus. But this > driver > doesn't have anything else ACPI-ish in it, so I decided that it > doesn't belong > under acpica/ or acpi_support/. Am I correct about this? > > Anything else you would like to report or comment or advise to me. > Thank you very much for your help. The driver looks good in general. A few questions: - Can you make the magic numbers a define ? Where did they come from ? - Are you missing a device_set_desc() call ? - If this is what you want to commit, C++ comments are not allowed per- style Regards, -- Rui Paulo
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5E0DD277-CAA3-4F01-8561-35CF6C511718>