From owner-freebsd-arm@freebsd.org Mon Jul 6 09:14:29 2015 Return-Path: Delivered-To: freebsd-arm@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 8A190729B5 for ; Mon, 6 Jul 2015 09:14:29 +0000 (UTC) (envelope-from daemon-user@freebsd.org) Received: from phabric-backend.isc.freebsd.org (phabric-backend.isc.freebsd.org [IPv6:2001:4f8:3:ffe0:406a:0:50:2]) by mx1.freebsd.org (Postfix) with ESMTP id 680691E86 for ; Mon, 6 Jul 2015 09:14:29 +0000 (UTC) (envelope-from daemon-user@freebsd.org) Received: by phabric-backend.isc.freebsd.org (Postfix, from userid 1346) id 648C3C023; Mon, 6 Jul 2015 09:14:29 +0000 (UTC) Date: Mon, 6 Jul 2015 09:14:29 +0000 To: freebsd-arm@freebsd.org From: "andrew (Andrew Turner)" Reply-to: D2378+327+81b4fa511783cd72@FreeBSD.org Subject: [Differential] [Updated] D2378: Introduce ITS support for ARM64 Message-ID: X-Priority: 3 Thread-Topic: D2378: Introduce ITS support for ARM64 X-Herald-Rules: <28>, <31>, <32>, <34>, <8> X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-Cc: X-Phabricator-Cc: X-Phabricator-Cc: X-Phabricator-Cc: X-Phabricator-Cc: X-Phabricator-Cc: X-Phabricator-Cc: In-Reply-To: References: Thread-Index: ZTc3NjU2YTNlNjQxNDIwYzYxMmJlZjFmYzJmIFWaRvU= Precedence: bulk X-Phabricator-Sent-This-Message: Yes X-Mail-Transport-Agent: MetaMTA X-Auto-Response-Suppress: All X-Phabricator-Mail-Tags: , MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="utf-8" X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.20 List-Id: "Porting FreeBSD to ARM processors." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Jul 2015 09:14:29 -0000 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