From owner-freebsd-arm@FreeBSD.ORG Tue Dec 29 13:53:38 2009 Return-Path: Delivered-To: freebsd-arm@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D534E1065696 for ; Tue, 29 Dec 2009 13:53:38 +0000 (UTC) (envelope-from rpaulo@gmail.com) Received: from mail-fx0-f227.google.com (mail-fx0-f227.google.com [209.85.220.227]) by mx1.freebsd.org (Postfix) with ESMTP id 657638FC18 for ; Tue, 29 Dec 2009 13:53:38 +0000 (UTC) Received: by fxm27 with SMTP id 27so10807992fxm.3 for ; Tue, 29 Dec 2009 05:53:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:sender:subject:mime-version :content-type:from:in-reply-to:date:cc:content-transfer-encoding :message-id:references:to:x-mailer; bh=hyT+gr5VTqijzWfDMUjqT2WnY+li+/k5lZ7EqDjE1XI=; b=ifExq6sqvf4ZlJmWqBl2Y+tWeBbRWVW+ttmPuCxvO+jfrND55rg6WP3PssorQM9ukp noP3h8BE1Jgdr16zdwlFhgYt0oZ+ighXRb06DVS8NVWuZuFuMDoGzUGDeXYaQg3Fx7kA kbFa1d+0r/uTxiX3Av7BEFNgmLULWVv4IaOkk= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=sender:subject:mime-version:content-type:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to:x-mailer; b=IoXLS2b5GOhCjBTDV3o39jXoNNMx8sRUddMroLpIvpREq7lS8T8OXnuelBEx8QE4el fpCZs671buUybXnYumh+lTKGjlKYMlQmhzp6/RgDNY1JX+vL4D6NTfW9PM1lbdChFYce bKKMucGXKj6MMhRl5Ge09b70mrWwfZyxPiGFU= Received: by 10.223.144.195 with SMTP id a3mr10500749fav.103.1262094813851; Tue, 29 Dec 2009 05:53:33 -0800 (PST) Received: from ?10.0.10.2? (54.81.54.77.rev.vodafone.pt [77.54.81.54]) by mx.google.com with ESMTPS id 13sm4294888fxm.9.2009.12.29.05.53.32 (version=TLSv1/SSLv3 cipher=RC4-MD5); Tue, 29 Dec 2009 05:53:33 -0800 (PST) Sender: Rui Paulo Mime-Version: 1.0 (Apple Message framework v1077) Content-Type: text/plain; charset=us-ascii From: Rui Paulo In-Reply-To: <260bb65e0912250948w6f714367w672a1ebf037fb7f7@mail.gmail.com> Date: Tue, 29 Dec 2009 13:53:31 +0000 Content-Transfer-Encoding: quoted-printable Message-Id: References: <260bb65e0912110627o6b67b399vabaae57477b91023@mail.gmail.com> <260bb65e0912250948w6f714367w672a1ebf037fb7f7@mail.gmail.com> To: Yohanes Nugroho X-Mailer: Apple Mail (2.1077) Cc: freebsd-arm@freebsd.org Subject: Re: CNS11XX FreeBSD port completed X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting FreeBSD to the StrongARM Processor List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 Dec 2009 13:53:39 -0000 Hi, On 25 Dec 2009, at 17:48, Yohanes Nugroho wrote: > Hi, >=20 > To make it easy for others to read the changes I have made, attached > is the diff version against SVN head. There is one change that may be > should not be commited. In vfs_mount.c, I added >=20 > pause("WAIT", hz * 10); >=20 > That line can be removed if the patch from this: >=20 > = http://lists.freebsd.org/pipermail/freebsd-current/2009-October/012361.htm= l >=20 > is applied Some comments: * please read style(9) and try to follow these guidelines on your code * did you forget to add a kernel config file for this board? * I would like for you to reconsider the code that's under C++ comments. = Either remove it or please use ANSI-style /* */ C comments. This is a = style issue. * please add your email to all the files you are adding (like you do on = if_ecereg.h, for example) * econa_machdep.c is a 4-claus BSD license. Can you check if we already = include "This product includes software developed by Brini." on our = documentation/marketing material? * there are some lines that go beyond 80 columns. Can you fix them? * in if_ece.c::poweron(), can't you use bus_read_4 instead of using a = volatile and pointing it to some specific memory address? * please avoid doing in-code variable declarations like "for (int i =3D = 0 ..." * what about the #if 0's ? * do you have the specs of the ethernet controller ? stuff like = "mac_port_config |=3D ((0x1 << 18));" would be better if "18" was a = define with an indication of what the bit actually does. Something like = "mac_port_config |=3D ((0x1 << MAC_PORT_0_DISABLE));" * there are many white lines that you can remove * defines should be like "#define...". This is especially visible = in econa_reg.h The rest looks fine. Thanks! -- Rui Paulo