Re: [ECOS] AMD Flash Driver Problem with CYGNUM_FLASH_16AS8

[Bart Veer <bartv at ecoscentric dot com>]

>>>>> "Jay" == Jay Foster <jay@systech.com> writes:

    Jay> I have several targets using the AMD flash driver with the
    Jay> following devices enabled:

    Jay> 	AM29LV081B, AM29LV017D, and AM29LV033C (S29AL032D).

    Jay> I want to add the AMD AM29LV160 (S29AL0160D) part, but ran
    Jay> into the following problem.

    Jay> Each of these flash devices is used in the 8-bit mode. For
    Jay> the existing parts, CYGNUM_FLASH_16AS8 needs to be defined as
    Jay> 0 (or left undefined). For the new part, CYGNUM_FLASH_16AS8
    Jay> needs to be defined as 1. Obviously, CYGNUM_FLASH_16AS8
    Jay> cannot be defined as two different values at the same time.

    Jay> It seems that this should also be parameterized in the flash
    Jay> parts table entries, so that these parts can be supported
    Jay> simultaneously. Has anyone else encountered this problem?

This seems a bit strange. The AM29LV081B is a 40-pin device. The
AM29LV160 is a 48-pin device. Do you have two boards, similar but not
identical, and you want to run the same application image on both?

In the V2 flash world this is fairly easy to support. You would
instantiate two CYG_FLASH_DRIVER() devices. One would use the _8
functions, the other _16as8. Both instances would probably use a CFI
init function to adapt to the actual h/w. Something like:

static const CYG_FLASH_FUNS(hal_alaia_flash_amd_funs0,
                            &cyg_am29xxxxx_init_cfi_8,
                            &cyg_am29xxxxx_query_nop,
                            &cyg_am29xxxxx_erase_8,
                            &cyg_am29xxxxx_program_8,
                            (int (*)(struct cyg_flash_dev*, const cyg_flashaddr_t, void*, size_t))0,
                            &cyg_am29xxxxx_hwr_map_error_nop,
                            &cyg_am29xxxxx_lock_nop,
                            &cyg_am29xxxxx_unlock_nop);


static const CYG_FLASH_FUNS(hal_alaia_flash_amd_funs1,
                            &cyg_am29xxxxx_init_cfi_16as8,
                            &cyg_am29xxxxx_query_nop,
                            &cyg_am29xxxxx_erase_16as8,
                            &cyg_am29xxxxx_program_16as8,
                            (int (*)(struct cyg_flash_dev*, const cyg_flashaddr_t, void*, size_t))0,
                            &cyg_am29xxxxx_hwr_map_error_nop,
                            &cyg_am29xxxxx_lock_nop,
                            &cyg_am29xxxxx_unlock_nop);


CYG_FLASH_DRIVER(hal_alaia_flash0,
                 &amp;hal_alaia_flash_amd_funs0,
                 ...
);

CYG_FLASH_DRIVER(hal_alaia_flash1,
                 &amp;hal_alaia_flash_amd_funs1,
                 ...
);

One of the init functions would fail, leaving that device instance
disabled. The other would succeed, and things should operate normally
from that point on.

This approach has some code and data overhead, you would be pulling in
two lots of flash manipulation functions and you need space for two
sets of flash data structures. However this overhead is only paid for
targets where you need to support both 8-bit and 16as8 support. For
the much more common case where only a single type of flash needs to
be supported there are no such overheads.

Your approach in the V1 flash world appears to add an extra field to
flash_dev_info_t and two lots of address arrays, unconditionally. So
you are adding overhead to all platforms that use an AMD device - even
though most of those platforms will not benefit from the extra
flexibility. That is undesirable. Hence although your patch solves the
specific problem you are trying to address, I do not think it is
appropriate for the mainline sources.

Bart

-- 
Bart Veer                       eCos Configuration Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss