From owner-freebsd-hackers@FreeBSD.ORG Mon Oct 19 11:23:43 2009 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B5391106566B; Mon, 19 Oct 2009 11:23:43 +0000 (UTC) (envelope-from avg@icyb.net.ua) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 8DEBB8FC21; Mon, 19 Oct 2009 11:23:42 +0000 (UTC) Received: from odyssey.starpoint.kiev.ua (alpha-e.starpoint.kiev.ua [212.40.38.101]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id OAA01763; Mon, 19 Oct 2009 14:23:40 +0300 (EEST) (envelope-from avg@icyb.net.ua) Message-ID: <4ADC4C3C.3000007@icyb.net.ua> Date: Mon, 19 Oct 2009 14:23:40 +0300 From: Andriy Gapon User-Agent: Thunderbird 2.0.0.23 (X11/20090825) MIME-Version: 1.0 To: Rui Paulo References: <4ADB764C.2010900@icyb.net.ua> <5E0DD277-CAA3-4F01-8561-35CF6C511718@freebsd.org> In-Reply-To: <5E0DD277-CAA3-4F01-8561-35CF6C511718@freebsd.org> X-Enigmail-Version: 0.95.7 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: freebsd-hackers@freebsd.org, freebsd-acpi@freebsd.org Subject: Re: SB7xx watchdog: new driver for review and testing X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 19 Oct 2009 11:23:43 -0000 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 >> > 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