From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2074 invoked by alias); 22 Apr 2012 16:34:33 -0000 Received: (qmail 1755 invoked by uid 22791); 22 Apr 2012 16:34:30 -0000 X-SWARE-Spam-Status: No, hits=-2.7 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,TW_EG 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; Sun, 22 Apr 2012 16:34:16 +0000 Received: from localhost (hagrid.ecoscentric.com [127.0.0.1]) by mail.ecoscentric.com (Postfix) with ESMTP id 129412F78012 for ; Sun, 22 Apr 2012 17:34:15 +0100 (BST) 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 5JIT5iaDGiPs; Sun, 22 Apr 2012 17:34:10 +0100 (BST) From: bugzilla-daemon@bugs.ecos.sourceware.org To: ecos-patches@ecos.sourceware.org Subject: [Bug 1001561] Internal flash driver for Freescale TWR-K60N512 board 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: ilijak@siva.com.mk X-Bugzilla-Status: UNCONFIRMED X-Bugzilla-Priority: low X-Bugzilla-Assigned-To: unassigned@bugs.ecos.sourceware.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Changed-Fields: CC 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: Sun, 22 Apr 2012 16:34:00 -0000 Message-Id: <20120422163410.405132F78001@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-04/txt/msg00053.txt.bz2 Please do not reply to this email. Use the web interface provided at: http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001561 Ilija Kocho changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |ilijak@siva.com.mk --- Comment #6 from Ilija Kocho 2012-04-22 17:34:01 BST --- (In reply to comment #5) > Hi Ilija > > I sent the e-mail requested for the copyright assignment and I am waiting for > their answer. Thanks, then, let's begin and prepare your code for commit while paperware is traveling. > I updated the attachments : > > - I move the common code to the variant level. > > - I separated them as you recommended. > > - I deleted the temporary files (sorry about that). > > Also I figured out why I wasn't able to access some part of the flash. In fact > speculation logic and cache aliasing (which are enable by default) are not > supported on some tower boards. See there for more informations : > http://forums.freescale.com/t5/Kinetis-ARM-Cortex-M4/Accessing-the-FLASH-causing-BUS-fault/m-p/78873#M509 Thank you for the pointer. > > I added an option to disable these options in the cdl file. The driver should > now work properly. Good usage of eCos configureability. I haven't yet tried the driver on target, here are some notes based on code review. First some formal notes: I should have pointed this earlier (sory), here you will find some guidance how to prepare eCos code: http://ecos.sourceware.org/patches-criteria.html That being said here are few notes: Update ChangeLog in existing packages (i.e. Kinetis HAL) and add one to new ones (the driver). Convert tabs to spaces (tab size 4 is OK). Make sure that Copyright banners are accurate. This includes year(s) in Copyright line. Line length should be typically less than 78 characters. This is a soft rule but please try to reformat your code accordingly. In file devs/flash/freescale/kinetis/current/src/kinetis_flash.c: function kinetis_flash_init(): Your code seems correct, but FYI eCos provides a set of hardware abstraction macros such as HAL_READ_UINT32() HAL_WRITE_UINT32() for hardware abstraction. Device registers are typically accessed either structured (as you do for cyghwr_hal_kinetis_flash_t) or by means of HAL macro. Would you mind editing this part? Example: cyg_uint32 regval; HAL_READ_UINT32(CYGHWR_HAL_KINETIS_FMC_PFB0CR, regval); regval &= ~CYGHWR_HAL_KINETIS_FMC_PFBCR_BIPE; HAL_WRITE_UINT32(CYGHWR_HAL_KINETIS_FMC_PFB0CR, regval); Lines like following: CYGHWR_HAL_KINETIS_SIM_P->scgc6 |= 0x01UL; There are macros for SCGS bit fields in var_io.h that you could use (If some macro is missing feel free to add) Note: FTFL clock is already being enabled during system init (all clock are) but do not remove this line from FTFL driver, in some future upgrade all drivers should have such and I intend to remove general clock enabling. I guess that flashCommandSequence() could be static. eCos is being linked to application(s) so let's keep exported names to minimum. Could query[] be more general: "Freescale Kinetis internal flash" or "Kinetis FTFL" ? In file devs/flash/freescale/kinetis/current/src/kinetis_flash.h: KINETIS_FLASH_BLOCK_SIZE: There are some devices with block size of 4KiB so it should be configurable or inferred from device type. I have been informed that there is some ambiguity in K60 devices as there are devices with both 2 KiB and 4 KiB block size, so to be on a safe side I would provide a CDL: cdl_option CYGNUM_DEVS_KINETIS_FLASH_BLOCK_SIZE { display "Block size" legal_values { 0 0x800 0x1000 } default_value 0 description ".... 0 - Auto ...." } In addition the platform HAL can override. Also check if some other value can be configurable/inferred or read from Kinetis registers. Regarding configuration options it would be good to cross-reference other device manuals, typically K40 and K70 (that we have or will have soon in eCos). Ilija -- 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.