[Differential] [Updated] D2378: Introduce ITS support for ARM64

andrew (Andrew Turner) phabric-noreply at FreeBSD.org
Mon Jul 6 09:14:29 UTC 2015


andrew added a comment.

Can you upload a patch with more context? If you're not using arc https://wiki.freebsd.org/CodeReview explains how to do it with svn and git.


INLINE COMMENTS
  sys/arm64/arm64/gic_v3_fdt.c:116 This is an odd place for the prototype.
  sys/arm64/arm64/gic_v3_fdt.c:140 You should be checking the return value, e.g.
    if (gic_v3_ofw_bus_attach(dev) != 0) {
  sys/arm64/arm64/gic_v3_fdt.c:199-200 Is this because it hasn't been added to the code, or because of that the fdt bindings say?
  sys/arm64/arm64/gic_v3_fdt.c:280 These should be collected at the start of the file.
  sys/arm64/arm64/gic_v3_fdt.c:299 Why is this here? Normally we put the probe function early on in the file.
  sys/arm64/arm64/gic_v3_its.c:671 Why do we need a full barrier, the comment should explain why.
  sys/arm64/arm64/gic_v3_its.c:818 Why do we need to flush the cache?
  sys/arm64/arm64/gic_v3_its.c:821 Why do we need a memory barrier?
  sys/arm64/arm64/gic_v3_its.c:1077-1080 Can you find out why? Marc Zyngier did the Linux driver and might be able to tell you why they have such a timeout.
  sys/arm64/arm64/gic_v3_its.c:162 Extra brace
  sys/arm64/arm64/gic_v3_its.c:686 Why do we need a full barrier here? The comment doesn't explain why.
  sys/arm64/arm64/gic_v3_its.c:840 Why do we need a cache wb or dsb? A comment explaining why would be useful.

REVISION DETAIL
  https://reviews.freebsd.org/D2378

EMAIL PREFERENCES
  https://reviews.freebsd.org/settings/panel/emailpreferences/

To: zbb, imp, ian, emaste, manpages, andrew
Cc: eadler, gnn, kib, emaste, andrew, freebsd-arm-list, imp


More information about the freebsd-arm mailing list