From owner-freebsd-arm@FreeBSD.ORG Wed Apr 22 09:27:44 2015 Return-Path: Delivered-To: freebsd-arm@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 2FBCAF9A for ; Wed, 22 Apr 2015 09:27:44 +0000 (UTC) Received: from phabric-backend.isc.freebsd.org (phabric-backend.isc.freebsd.org [IPv6:2001:4f8:3:ffe0:406a:0:50:2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 0882A1798 for ; Wed, 22 Apr 2015 09:27:44 +0000 (UTC) Received: from phabric-backend.isc.freebsd.org (phabric-backend.isc.freebsd.org [127.0.1.5]) by phabric-backend.isc.freebsd.org (8.14.9/8.14.9) with ESMTP id t3M9RhIR018192 for ; Wed, 22 Apr 2015 09:27:43 GMT (envelope-from eadler@phabric-backend.isc.freebsd.org) Received: (from root@localhost) by phabric-backend.isc.freebsd.org (8.14.9/8.14.9/Submit) id t3M9RhMc018191; Wed, 22 Apr 2015 09:27:43 GMT (envelope-from eadler) Date: Wed, 22 Apr 2015 09:27:43 +0000 To: freebsd-arm@freebsd.org From: "andrew (Andrew Turner)" Subject: [Differential] [Updated] D2340: Support for Alpine platform from Annapurna Labs Message-ID: X-Priority: 3 Thread-Topic: D2340: Support for Alpine platform from Annapurna Labs X-Herald-Rules: none X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-Cc: In-Reply-To: References: Thread-Index: MzFmODEzYTliMzhlZDlhOTEyM2NlYTg2YmJiIFU3aY8= 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: Wed, 22 Apr 2015 09:27:44 -0000 andrew added a comment. First pass though the change. INLINE COMMENTS sys/arm/annapurna/alpine/alpine_machdep.c:144 What is this for, and why does it need a fixed virtual address? sys/arm/annapurna/alpine/alpine_machdep.c:169 These should use the functions in sys/arm/annapurna/alpine/alpine_machdep_mp.c:91 You should use ofw_cpu_early_foreach for this. sys/arm/annapurna/alpine/alpine_pci.c:310 What is this magic number? And, in general the style is: ``` if ((cell0 & MAGIC_VALUE) == MAGIC_VALUE) ``` or: ``` if ((cell0 & MAGIC_VALUE) != 0) ``` sys/arm/annapurna/alpine/alpine_pci.c:372 What's special about 255? sys/arm/annapurna/alpine/alpine_pci.c:375 And why are you shifting these variables by this amount? sys/arm/annapurna/alpine/alpine_pci.c:399 Workaround for what? sys/arm/annapurna/alpine/alpine_pci.c:477 Don't use fdt_ functions here, use the ofw_ version sys/arm/annapurna/alpine/alpine_pci.c:494 These should be using ofw_bus_search_compatible, and the probe functions shouldn't be setting bits of the softc, move those to the attach function. sys/arm/annapurna/alpine/alpine_pci.c:762 This is an odd spelling of powerof2(n) sys/arm/annapurna/alpine/alpine_pci.c:904 What's special about 5? sys/arm/annapurna/alpine/alpine_pci.c:980 What's special about 12? sys/arm/annapurna/alpine/alpine_pci.c:1063 What are all these magic numbers? sys/arm/annapurna/alpine/alpine_pci.c:1476 I assume 4 as some meaning? sys/arm/annapurna/alpine/files.alpine:11 you don't need the cpufunc_asm_ files here. sys/arm/annapurna/alpine/std.alpine:9 I don't think this is needed. sys/arm/annapurna/alpine/std.alpine:12 You only need to set KERNPHYSADDR if you need kernel.bin sys/arm/annapurna/alpine/std.alpine:15 This will probably cause issues with loader. sys/arm/conf/ALPINE:50 Can you try to update this file to be closer to the other armv6 configs. for example the above is missing a number of options. sys/arm/conf/ALPINE:64 Why is this hard coded? sys/arm/conf/ALPINE:110 Do you need a static dtb? sys/boot/fdt/dts/arm/annapurna-alpine.dts:2 How similar is this to the upstream dts? We are aiming to use dts files as close to the Linux version as possible. REVISION DETAIL https://reviews.freebsd.org/D2340 EMAIL PREFERENCES https://reviews.freebsd.org/settings/panel/emailpreferences/ To: jpa-semihalf.com, ian, imp, andrew Cc: freebsd-arm