Date: Mon, 19 Oct 2009 14:23:40 +0300 From: Andriy Gapon <avg@icyb.net.ua> To: Rui Paulo <rpaulo@freebsd.org> Cc: freebsd-hackers@freebsd.org, freebsd-acpi@freebsd.org Subject: Re: SB7xx watchdog: new driver for review and testing Message-ID: <4ADC4C3C.3000007@icyb.net.ua> In-Reply-To: <5E0DD277-CAA3-4F01-8561-35CF6C511718@freebsd.org> References: <4ADB764C.2010900@icyb.net.ua> <5E0DD277-CAA3-4F01-8561-35CF6C511718@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
on 19/10/2009 14:17 Rui Paulo said the following: > 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 ? Yes, will do this. The numbers are from register definitions in AMD SB700/710/750 Register Reference Guide: http://developer.amd.com/assets/43009_sb7xx_rrg_pub_1.00.pdf I will add a link to the document too. > - Are you missing a device_set_desc() call ? Yes, I missed this. Thanks! > - If this is what you want to commit, C++ comments are not allowed > per-style Those lines were a result of quick hacking. I will remove them altogether, -- Andriy Gapon
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4ADC4C3C.3000007>