Date: Tue, 21 Apr 2015 17:03:32 +0000 From: "imp (Warner Losh)" <phabric-noreply@FreeBSD.org> To: freebsd-arm@freebsd.org Subject: [Differential] [Updated] D2340: Support for Alpine platform from Annapurna Labs Message-ID: <f73163090a8e1db44a0356521d9892ec@localhost.localdomain> In-Reply-To: <differential-rev-PHID-DREV-gclxe3zq7xamd5hpjagv-req@FreeBSD.org> References: <differential-rev-PHID-DREV-gclxe3zq7xamd5hpjagv-req@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
imp added a comment. Decent start, want to know more about the future of the HAL. Is this a one-off never to be repeated HAL release, or will there be ongoing releases? INLINE COMMENTS sys/arm/annapurna/alpine/alpine_machdep.c:2 Here and elsewhere: is this date right? sys/arm/annapurna/alpine/alpine_machdep.c:165 Perhaps this belongs in the main db_machdep.c? sys/arm/annapurna/alpine/alpine_machdep.c:199 ditto? Both with appropriate #ifdef maybe? sys/arm/annapurna/alpine/alpine_machdep_mp.c:143 Shouldn't this be a kassert? sys/arm/annapurna/alpine/alpine_machdep_mp.c:172 kassert? sys/arm/annapurna/alpine/alpine_pci.c:84 Given how this macro is used in this file, I might be tempted to make it available under bootverbose instead of 'DEBUG' sys/arm/annapurna/alpine/alpine_pci.c:667 ~0 is signed, but this takes an unsigned value. 0xfffffffful is likely a better match. sys/arm/annapurna/alpine/alpine_pci.c:1097 Why enable these explicitly here? Drivers are supposed to do so... sys/arm/annapurna/alpine/alpine_pci.c:1138 but here it's OK since the bridge is supposed to do this on enable... sys/arm/annapurna/alpine/alpine_pci.c:1372 why do this if it is unused? sys/arm/annapurna/alpine/alpine_pci.c:1378 ~3 seems magic. sys/arm/annapurna/alpine/alpine_pci.c:1387 Most places use NBBY instead of a bare 8 for 8 bits in a byte. sys/arm/annapurna/alpine/alpine_pci.c:1412 why get it? sys/arm/annapurna/alpine/hal/al_hal_iofic.h:1 For the HAL files, will we need to worry about updates? For Octeon/Cavium, we imported their SDK/HAL routines separately. Does it make sense to do that here? REVISION DETAIL https://reviews.freebsd.org/D2340 To: jpa-semihalf.com, ian, andrew, imp Cc: freebsd-arm
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?f73163090a8e1db44a0356521d9892ec>