This is the mail archive of the
ecos-patches@sourceware.org
mailing list for the eCos project.
[Bug 1001187] New port - HAL: Freescale Kinetis variant, TWR-K60N512, TWR-K40X256 plf; Devs: Freescale UART and ENET (Ethernet)
- From: bugzilla-daemon at bugs dot ecos dot sourceware dot org
- To: ecos-patches at ecos dot sourceware dot org
- Date: Mon, 17 Oct 2011 22:30:35 +0100
- Subject: [Bug 1001187] New port - HAL: Freescale Kinetis variant, TWR-K60N512, TWR-K40X256 plf; Devs: Freescale UART and ENET (Ethernet)
- Auto-submitted: auto-generated
- References: <bug-1001187-104@http.bugs.ecos.sourceware.org/>
Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001187
--- Comment #22 from Ilija Kocho <ilijak@siva.com.mk> 2011-10-17 22:30:28 BST ---
(In reply to comment #20)
> As per direct mail, here's a few comments, although I haven't done what I'd
> call a full review:
>
>
> - I'll just mention that if extra TTYs are justified, then you should just
> modify CYGPKG_IO_SERIAL.
This has some history. The idea was a provision for adding (as much as needed)
additional tty/termios drivers into target without the need for changing IO.
Luckily after a long session with Sergei the patch was surprisingly short.
Please see: http://ecos.sourceware.org/ml/ecos-devel/2011-03/msg00023.html
Also: Bug #1001180
>
> - "familly" should be spelt "family", and "acces" -> "access"
Thank you. Luckily there isn't wellcome (pardon welcome) or luckilly :) in my
text.
>
> - CDL display names shouldn't really end in full-stops ("."). The best way to
> decide what to use for display names is to think of what would be best in the
> graphical configuration tool - even if you, like many maintainers, don't use
> it, lots of eCos users do.
I'm trying to make the displays as appropriate as I can, but sometimes they
resemble sentences - hence full-stops. I'll clean them.
>
> - Various components which are "flavor none" should also have "no_define".
I don't know why, but I had impression that flavor none are no_define by
default... but now I know they aren't.
>
> - The CYGPKG_HAL_CORTEXM_KINETIS .cdl file is quite big - perhaps the
> components related to clocking could be split off into a separate file, and
> included.
Yeah. This clock is more complex than some 8-bit micro-controllers. Good idea.
This brings me an idea of extracting clocking code from kinetis_misc.c?
kinetis_clocking.cdl
kinetis_clocking.c
Any comment?
>
> - This bit in hal_diag.c:
> +#if defined(CYGOPT_HAL_KINETIS_DIAG_IN_MISC_FLASH_SECTION) && \
> + CYGOPT_HAL_KINETIS_DIAG_IN_MISC_FLASH_SECTION
> +# define CYGOPT_HAL_KINETIS_DIAG_FLASH_SECTION_ATTR \
> + CYGOPT_HAL_KINETIS_MISC_FLASH_SECTION_ATTR
> +#else
> +# define CYGOPT_HAL_KINETIS_DIAG_FLASH_SECTION_ATTR
> +#endif
>
> I think you originally meant to begin that with:
> +#if defined(CYGOPT_HAL_KINETIS_DIAG_IN_MISC_FLASH_SECTION) && \
> + CYGOPT_HAL_KINETIS_MISC_FLASH_SECTION_ATTR
>
It probably used to be flavor data or booldata defining section name. Than I
decided to simplify it (literal section name), but preprocessor code remained.
I'll look for eventual other cases.
> But given the CDL you can probably just use:
> #ifdef CYGOPT_HAL_KINETIS_DIAG_IN_MISC_FLASH_SECTION
>
>
> Nothing else leapt out at me. It seems a very good quality patch from what I
> can tell.
>
Thank you for good advices.
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.