Skip site navigation (1)Skip section navigation (2)
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>