This is the mail archive of the ecos-patches@sourceware.org mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Bug 1001561] Internal flash driver for Freescale TWR-K60N512 board


Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001561

--- Comment #9 from Nicolas Aujoux <nau@csm-instruments.com> 2012-05-02 09:27:12 BST ---
(In reply to comment #6)

Hi Ilija,

If I am not mistaken, the last patches should implement what you pointed out.

For the block size thing, I did some quick research but it seems that there is
no way to know the size by reading registers. 
For the k60, it looks like 100Mhz devices have 2KB block size and 120,150 MHz
have 4KB block size.

It also looks like the block size has an impact on how to program the flash : I
use the program longword command in my driver for the k60 100 MHz but this
command doesn't exist in the k60 120 or 150 MHz.

Nicolas

> (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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]