From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2160 invoked by alias); 13 Dec 2012 16:06:45 -0000 Received: (qmail 2114 invoked by uid 22791); 13 Dec 2012 16:06:39 -0000 X-SWARE-Spam-Status: No, hits=-2.7 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW 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; Thu, 13 Dec 2012 16:06:35 +0000 Received: from localhost (hagrid.ecoscentric.com [127.0.0.1]) by mail.ecoscentric.com (Postfix) with ESMTP id 7AD4F4680010 for ; Thu, 13 Dec 2012 16:06:34 +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 HuWrH8BvSWyM; Thu, 13 Dec 2012 16:06:23 +0000 (GMT) From: bugzilla-daemon@bugs.ecos.sourceware.org To: ecos-patches@ecos.sourceware.org Subject: [Bug 1001716] MIPS malta + sead3 update 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: enhancement X-Bugzilla-Who: jifl@ecoscentric.com X-Bugzilla-Status: NEEDINFO X-Bugzilla-Priority: low X-Bugzilla-Assigned-To: unassigned@bugs.ecos.sourceware.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Changed-Fields: Status CC Ever Confirmed 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: Thu, 13 Dec 2012 16:06:00 -0000 Message-Id: <20121213160623.C3D96468000E@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: 2012-12/txt/msg00007.txt.bz2 Please do not reply to this email. Use the web interface provided at: http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001716 Jonathan Larmour changed: What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |NEEDINFO CC| |jifl@ecoscentric.com Ever Confirmed|0 |1 --- Comment #1 from Jonathan Larmour 2012-12-13 16:06:22 GMT --- Thanks for contributing back! The first thing that will be needed to allow this to be checked in to the repository is a copyright assignment from MIPS to the FSF. Please read http://ecos.sourceware.org/assign.html and fill in the form linked from the there and submit it to the FSF. In terms of the patch itself, a few things that stand out that would want addressing, some are vital licensing issues, some are to match existing eCos coding standard practice: - You need ChangeLog entries for all changes - New files should have the standard form of file banners at the start we use, and in preparation for the assignment being completed should be marked as Copyright the Free Software Foundation. Obviously, refer to any other files for examples. The .ecm files, essentially being generated, can have their copyright line removed. - I suspect you would not want to assign copyright of msc01_pci.h to the FSF as it comes from elsewhere. In which case it should still have a standard eCos license/information banner at the beginning, but MIPS will also have to provide a license which this file may be used under, which must be compatible with the GPL. My suggestion would be either the GNU All-permissive license: http://www.gnu.org/prep/maintain/html_node/License-Notices-for-Other-Files.html or the modified BSD license: http://directory.fsf.org/wiki/License:BSD_3Clause - CYGPKG_MALTA_QEMU and CYGPKG_SEAD3 need descriptions. - CYGPKG_IO_MALTA_SERIAL should be an option not a component. - Naming of globals and functions should either have a cyg_ or hal_ prefix. For example the functions uart_*, malta_*, arch_pci_config_access(), - Can you explain the change in memory address of mlt_mips_malta_ram.ldi ? If nothing else, this will cause compatibility problems with existing ROM monitors. Does it reflect a hardware change? If so, we would need existing hardware to continue working. - The use of uart2_channel looks a lot like a bodge. There are two serial devices after all. I think this wants improvement. If you aren't able to do it (short of time), it should be submitted into bugzilla as a bug report so the issue is tracked and not forgotten. - uart_debug and uart2_printf should be removed - standard HAL APIs should be used instead. - Is there really a need for the extra .ld files? Can't you just use ifdefs in the normal mips_mips32.ld file? That will do for a first pass of comments I think. We'll wait for the assignment to come through before anything else. Jifl -- 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.