SB7xx watchdog: new driver for review and testing

[Andriy Gapon]

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