Page MenuHomeFreeBSD

wbwd: move to superio(4) bus
ClosedPublic

Authored by avg on Oct 11 2019, 11:21 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Oct 29, 9:52 PM
Unknown Object (File)
Sep 30 2024, 1:15 PM
Unknown Object (File)
Sep 17 2024, 11:01 AM
Unknown Object (File)
Sep 8 2024, 2:23 PM
Unknown Object (File)
Sep 8 2024, 6:27 AM
Unknown Object (File)
Sep 7 2024, 10:57 PM
Unknown Object (File)
Sep 7 2024, 6:40 PM
Unknown Object (File)
Sep 5 2024, 9:59 AM
Subscribers

Details

Summary

This allows to remove a bunch of low level code.
Also, superio(4) provides safer interaction with other drivers
that work with Super I/O configuration registers.

Test Plan

Tested only on PCengines APU2:
superio0: <Nuvoton NCT5104D/NCT6102D/NCT6106D (rev. B+)> at port 0x2e-0x2f on isa0
wbwd0: <Nuvoton NCT6102 (0xc4/0x53) Watchdog Timer> at WDT ldn 0x08 on superio0

The watchdog output is incorrectly wired on that system and the watchdog
does not really do it its job, but the pulse can be seen with a signal
analyzer.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bcr added a subscriber: bcr.

OK from manpages.

Looks fine to me, but I no longer have the hardware to actually test the changes, iXsystems may be able to help with that.

There are a few minor nits (commented inline) and these are all optional (most of these are style changes), the only real ask from me is to provide some level of visibility when the probed device is unsupported, at least in bootverbose mode instead of a silent ENXIO return.

sys/dev/wbwd/wbwd.c
6 ↗(On Diff #63144)

[OPTIONAL ]The change was close to 1/2 and you deserve to be added here.

313 ↗(On Diff #63144)

[OPTIONAL] This whole block is not really used and can probably be removed altogether.

467 ↗(On Diff #63144)

OPTIONAL: style(9) wants variables to be ordered by size, then in alphabetical order, so this should be:

uint8_t devid, revid;
int j;
struct wb_softc *sc;
char buf[128];

Note that 'found' is unused and can be removed. I'd probably replace 'j' with 'i' since i no longer exist.

492 ↗(On Diff #63144)

OPTIONAL: It's probably worthy to print out the dev_id and dev_rev here in bootverbose?

507 ↗(On Diff #63144)

OPTIONAL: I'd probably refactor this to a switch/case, but I'm fine with the code as-is too.

This revision is now accepted and ready to land.Oct 11 2019, 10:02 PM
This revision was automatically updated to reflect the committed changes.
sys/dev/wbwd/wbwd.c
467 ↗(On Diff #63144)

FWIW, I always read that style(9) guideline as saying that larger variables should come first.