Re: TS1000 port added

[Jonathan Larmour <jifl at eCosCentric dot com>]

Gary Thomas wrote:
> New port, based on PowerPC 855T, diffs attached.
Just two little things first: can you add this to the NEWS file too 
please? If a company funded the work and they are willing, they can be 
credited too if they like. And either give me the info, or go ahead and 
update the entry that would be at 
<http://sources.redhat.com/ecos/hardware.html>

I know there's already missing targets, but best not to compound it.

> package CYGPKG_DEVS_FLASH_TS1000 {

<analjifl>
Shouldn't this be CYGPKG_DEVS_FLASH_POWERPC_TS1000 for consistency with 
the others? Or is it a generic board with CPU modules a la cogent? (I 
didn't think it was).
</analjifl>

> +        else {
> +            os_printf("** FEC Info: PHY LINK alreay UP \n");

I fixed the typo.

//      ts1000_flash.c
+#define CYGNUM_FLASH_INTERLEAVE	(1)
+#define CYGNUM_FLASH_SERIES	(1)
+#define CYGNUM_FLASH_WIDTH      (16)
+#define CYGNUM_FLASH_BASE 	(0xFE000000)
+// #define CYGNUM_FLASH_16AS8      (0)

Not important enough to go back and fix, but be wary of TABs. <old saw> 
They should really be spaces to help prevent editor confusion, patch 
mangling in e-mails or problems like the above.</old saw>

+//      plf_cache.h
+//-----------------------------------------------------------------------------
+// FIXME: This definition forces the IO flash driver to use a
+// known-good procedure for fiddling flash before calling flash device
+// driver functions. The procedure breaks on other platform/driver
+// combinations though so is depricated. Hence this definition.
+//
+// If you work on this target, please try to remove this definition
+// and verify that the flash driver still works (both from RAM and
+// flash). If it does, remove the definition and this comment for good
+// [and the old macro definition if this happens to be the last client
+// of that code].
+#define HAL_FLASH_CACHES_OLD_MACROS

Seems suboptimal to keep this around. By the sound of it, we don't want to 
propagate this if at all possible.

+//      hal_diag.c
[snip]
+#if !defined(CYGSEM_HAL_VIRTUAL_VECTOR_DIAG)
[snip]
+EPPC *eppc;

I think if this code isn't used it should just be snipped, lest it confuse 
someone trying to debug stuff. Ditto the associated bits in hal_diag.h.

Jifl
--
eCosCentric       http://www.eCosCentric.com/       <info@eCosCentric.com>
--[ "You can complain because roses have thorns, or you ]--
--[  can rejoice because thorns have roses." -Lincoln   ]-- Opinions==mine