[Bug 1001428] Hal bits for Kinetis K40 SLCD controller

[bugzilla-daemon at bugs dot ecos dot sourceware dot org]

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

--- Comment #1 from Ilija Kocho <ilijak@siva.com.mk> 2011-12-21 11:03:02 GMT ---
Hi Tomas

(In reply to comment #0)
> Created an attachment (id=1476)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1476) [details]
> Hal bits for K40 LCD controller
> 
> The attached patch adds register definitions and manipulation macros for the
> SLCD controller on the K40 chips; I think the same controller is also on the
> K30 and some of the K5? chips, but I have not verified this in detail, so the
> definitions are only included for the K40 subfamily at the moment.
> 
> (As a side note, I am working on a driver the the LCD on the kwikstik board; I
> will add that to bug 1001426 when it's ready.)

Regarding your activity as a new (AFAIK) and productive eCos contributor
here are some general / introductory remarks (also applicable to Bug 1001397
which I hope to visit soon)  :

Regarding device drivers and HAL: In eCos terms, "HAL" refers to software
associated with CPU and hardware close to it. Therefore devices are not
considered part of HAL. The idea is to re(use) drivers with different
architectures. In practice there are many exceptions but I would like to keep
at least Kinetis on this course. There are good reasons for this, one being the
fact that Freescale provides same or compatible peripherals on different
architectures.
I am trying to make drivers as much as possible independent of Kinetis HAL and
one aspect is name space. Namely, for device naming I have used FREESCALE stem
rather than KINETIS, DEV or IO instead of HAL. Also instead of using
/devs/<xxx>/cortexm I have used/crated /devs/<xxx>/freescale directories.
I would invite you to keep this conventions with hope that in future someone
may (wish to) use the drivers on Coldfire+ or PX.


Regarding bits and shifts: Like it is in Bug 1001388, it would be good to use
BIT_() and VALUE_() macros for bits and shifts. Code looks cleaner and Cortex-M
eCos users are used to them. A minor (personal) note - I find "u" suffix on hex
constants unnecessary (though it might have been recommended by some coding
standards).


Regarding register macros such as:
#define CYG_HAL_KINETIS_LCD_GCR \
  ((cyghwr_hal_kinetis_lcd_t*)CYG_HAL_KINETIS_LCD_B)->gcr

IMO such macros tend to produce sub-optimal code and I wouldn't accept them.
Commonly, you need same pointer more than once within a function (to access
same or different structure member(s)) and it is more effective to declare a
pointer and (re)use it.
Should you prefer to operate with register-like API, eCos has provision for
this by means of  HAL_WRITE_UINTxx() HAL_WRITE_READxx() macros - please look
ENET and UART drivers for sample usage. I would have to point that this API is
preferred choice by original eCos authors. In my eyes both ways are good, only
it would be bad idea to mix them on a same device.

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.