From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6346 invoked by alias); 22 Mar 2011 15:09:13 -0000 Received: (qmail 6331 invoked by uid 22791); 22 Mar 2011 15:09:11 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from hagrid.ecoscentric.com (HELO mail.ecoscentric.com) (212.13.207.197) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 22 Mar 2011 15:08:36 +0000 Received: from localhost (hagrid.ecoscentric.com [127.0.0.1]) by mail.ecoscentric.com (Postfix) with ESMTP id ABF212F78008 for ; Tue, 22 Mar 2011 15:08:33 +0000 (GMT) Received: from mail.ecoscentric.com ([127.0.0.1]) by localhost (hagrid.ecoscentric.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id xBdE+bnksuIW; Tue, 22 Mar 2011 15:08:28 +0000 (GMT) From: bugzilla-daemon@bugs.ecos.sourceware.org To: ecos-patches@ecos.sourceware.org Subject: [Bug 1000819] Add support for Atmel AT91SAM9263 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: eCos X-Bugzilla-Component: Patches and contributions X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: jifl@ecoscentric.com X-Bugzilla-Status: ASSIGNED X-Bugzilla-Priority: normal X-Bugzilla-Assigned-To: john@dallaway.org.uk X-Bugzilla-Target-Milestone: --- X-Bugzilla-Changed-Fields: In-Reply-To: References: X-Bugzilla-URL: http://bugs.ecos.sourceware.org/ Auto-Submitted: auto-generated Content-Type: text/plain; charset="UTF-8" MIME-Version: 1.0 Date: Tue, 22 Mar 2011 15:09:00 -0000 Message-Id: <20110322150828.9DBA12FB000A@mail.ecoscentric.com> Mailing-List: contact ecos-patches-help@ecos.sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: ecos-patches-owner@ecos.sourceware.org X-SW-Source: 2011-03/txt/msg00054.txt.bz2 Please do not reply to this email. Use the web interface provided at: http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000819 --- Comment #23 from Jonathan Larmour 2011-03-22 15:08:26 GMT --- (In reply to comment #22) > > a) Is the separation of PIO layout definitions into separate header files > implemented at the correct level in this case (processor level)? Yes. > b) Does it make sense to separate the PIO layout definitions from other I/O > definitions (if any) in this way? I think it would be good to also separate other I/O definitions. I don't know if that's too much to ask at the moment, so starting with PIO may well be fine. You don't _want_ e.g. all SAM7x definitions put in one single file - you want something more modular in any case because there will be commonality with other processors. > c) For the existing ports, would it be preferable to place the PIO layout > definitions in the processor HAL rather than in the AT91 variant HAL? This > would avoid the need to give each PIO layout header file a unique name. We need > to weigh up the risk of breaking platform ports we cannot readily test. I think in the current patch, the new pio_sam7*.h may as well live in the at91sam7s hal, to keep all the sam7 stuff together. Of course non-SAM7's don't have a separate processor HAL, and I don't think it's worth changing that at this point. As you say in (d), future processor HALs, e.g. most obviously the SAM9 should have its pio_sam9*.h files in it. IMO. I don't think having unique names is a big deal in itself. FAOD I think it is important to keep it as a define, rather than /requiring/ there to be a file with a particular name. > d) For new ports (including AT91SAM9 family), would it be preferable to place > the PIO layout definitions in the processor HAL rather than in the AT91 variant > HAL? I definitely think so. Yes. > Comments? > > Any other issues relating specifically to patch 3? Yes, the biggest problem is that there are (again) no copyright headers for new files, so formally I need to reject the patch as it stands. But the above comments will require changes anyway. -- Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug.