SB7xx watchdog: new driver for review and testing

Rui Paulo rpaulo at freebsd.org
Mon Oct 19 11:17:27 UTC 2009


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



More information about the freebsd-acpi mailing list