Date: Mon, 6 Jul 2015 09:14:29 +0000 From: "andrew (Andrew Turner)" <phabric-noreply@FreeBSD.org> To: freebsd-arm@freebsd.org Subject: [Differential] [Updated] D2378: Introduce ITS support for ARM64 Message-ID: <bfaeefa3c9b15bf23ac73ac3048f9396@localhost.localdomain> In-Reply-To: <differential-rev-PHID-DREV-xmqmcfafyun443233vly-req@FreeBSD.org> References: <differential-rev-PHID-DREV-xmqmcfafyun443233vly-req@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bfaeefa3c9b15bf23ac73ac3048f9396>