From owner-freebsd-acpi@FreeBSD.ORG Mon Oct 19 11:17:26 2009 Return-Path: Delivered-To: freebsd-acpi@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D04411065679; Mon, 19 Oct 2009 11:17:26 +0000 (UTC) (envelope-from rpaulo@gmail.com) Received: from mail-fx0-f218.google.com (mail-fx0-f218.google.com [209.85.220.218]) by mx1.freebsd.org (Postfix) with ESMTP id 38E558FC18; Mon, 19 Oct 2009 11:17:25 +0000 (UTC) Received: by fxm18 with SMTP id 18so4902497fxm.37 for ; Mon, 19 Oct 2009 04:17:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:sender:subject:mime-version :content-type:from:in-reply-to:date:cc:content-transfer-encoding :message-id:references:to:x-mailer; bh=2XJ3S1ZDhInOZK5OjBecgSoAnc3Q5PuamF509Z4i/EU=; b=KUKSTbRJazWtkcLoRppIzS4Ct3VNEp3V2cOmdJ03C99lh7kpxnaZApx0lC6GflVrRH IzMKFhlDj0PTmWIpt8YKRQ6r7a9pBHUIy80I+BRthzmh+KTYzLkdhvj1E5URq11FiIWh RyAHd4bfV3ri6xm5c3GcasPi3HLDGDHynTfxo= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=sender:subject:mime-version:content-type:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to:x-mailer; b=Oj1Qu6bu+xFa/1BrOx13hcZcPmIbwJRJ+TJJb/ErM5av6NBRQd+L99CFPPgU+uVnfX wfruC0pQbjlRwUxM2xtvjYz+b+iukQtp3etxQLee9wuE5cwyuc7pGR69NCqs5QAGT1CB s5hfZGXn8zV3KeLcJvqy8e6EkLMhaGj6mQSWQ= Received: by 10.204.15.3 with SMTP id i3mr4744738bka.71.1255951045080; Mon, 19 Oct 2009 04:17:25 -0700 (PDT) Received: from mac-mini.lan (bl6-159-136.dsl.telepac.pt [82.155.159.136]) by mx.google.com with ESMTPS id 13sm589223bwz.14.2009.10.19.04.17.23 (version=TLSv1/SSLv3 cipher=RC4-MD5); Mon, 19 Oct 2009 04:17:24 -0700 (PDT) Sender: Rui Paulo Mime-Version: 1.0 (Apple Message framework v1076) Content-Type: text/plain; charset=us-ascii; format=flowed; delsp=yes From: Rui Paulo In-Reply-To: <4ADB764C.2010900@icyb.net.ua> Date: Mon, 19 Oct 2009 12:17:22 +0100 Content-Transfer-Encoding: 7bit Message-Id: <5E0DD277-CAA3-4F01-8561-35CF6C511718@freebsd.org> References: <4ADB764C.2010900@icyb.net.ua> To: Andriy Gapon X-Mailer: Apple Mail (2.1076) Cc: freebsd-hackers@freebsd.org, freebsd-acpi@freebsd.org Subject: Re: SB7xx watchdog: new driver for review and testing X-BeenThere: freebsd-acpi@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: ACPI and power management development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 19 Oct 2009 11:17:27 -0000 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 ? - 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