Page MenuHomeFreeBSD

Allwinner A31 Watchdog
ClosedPublic

Authored by manu_bidouilliste.com on Feb 18 2016, 5:54 PM.
Tags
Referenced Files
Unknown Object (File)
Feb 22 2024, 5:56 PM
Unknown Object (File)
Dec 23 2023, 10:38 PM
Unknown Object (File)
Dec 22 2023, 9:06 PM
Unknown Object (File)
Nov 13 2023, 10:57 AM
Unknown Object (File)
Oct 24 2023, 9:19 PM
Unknown Object (File)
Sep 29 2023, 7:13 PM
Unknown Object (File)
Sep 18 2023, 8:49 AM
Unknown Object (File)
Sep 15 2023, 12:52 PM

Details

Summary

Make a10 watchdog driver more generic and add support to A31 watchdog.
Those are actually the only two watchdog on Allwinner SoC.
It is needed to add support to A31/A83/H3 (for which supports in on it s way).

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 2553
Build 2570: arc lint + arc unit

Event Timeline

manu_bidouilliste.com retitled this revision from to Allwinner A31 Watchdog.
manu_bidouilliste.com updated this object.
manu_bidouilliste.com edited the test plan for this revision. (Show Details)
manu_bidouilliste.com set the repository for this revision to rS FreeBSD src repository - subversion.
manu_bidouilliste.com added a project: ARM.
sys/arm/allwinner/aw_wdog.c
54–60

There is some odd indentation here.

122–131

You should use ofw_bus_search_compatible, and set the a10 flag in the attach function.

154

I would suggest putting this last, after all the softc data is set.

sys/arm/allwinner/aw_wdog.c
54–60

This is the original indentation but I can fix that if you want.

sys/arm/allwinner/aw_wdog.h
28–29

This should be changed to __AW_WDOG_H__

sys/arm/allwinner/aw_wdog.c
55

Bits 1:0 of A31_WDOG_CONFIG controls the target for the reset signal. 0x1 is "to whole system" and 0x2 is "only interrupt". Maybe A31_WDOG_CONFIG_RST_EN could have a name to better reflect this?

Use ofw_bus_search_compatible.
Rename A31_WDOG_CONFIG_RST_EN to A31_WDOG_CONFIG_RST_EN_SYSTEM.
Rename ifdef in aw_wdog.h
Fix indentation to reflect style(9).
Remove a10 var in softc as it is not needed anymore

sys/arm/allwinner/aw_wdog.c
54–60

I normally use #define<tab>FOO for the register and #define<tab> FOO_BAR for the contents of the register.

94–99

At least wdog_mode_en should be a 32-bit value as it stores the contents of the register. It could be argued the register offsets should be bus_size_t. Given there are only a limited number of copies of this struct the size shouldn't matter too much.

130

Extra return.

131

Missing device_set_desc(dev, "Allwinner A10 Watchdog"); (or similar).

155

You could put a unique value in the compat_data map, then have something like:

switch (ofw_bus_search_compatible(dev, compat_data)->ocd_data) {
case A10:
   ...
   break;
case A31:
   ...
   break;
 default:
   ...
   return (ENXIO);
}
169

If you move the mtx_init call below this you can remove this line.

Fix style(9) on defines.
Add A10_WATCHDOG, A31_WATCHDOG defines and use them in the compat_data.
Set device desc based on the compat data which matched.

This revision is now accepted and ready to land.Feb 25 2016, 5:02 PM