On Tuesday 29 April 2008, Vasiliy Leoenenko wrote:
The first patch (support of long commands):
I like the idea of splitting this patch up in two separate patches/emails. But please provide a descriptive subject and a short description that can will be added to the git repository as commit text for each patch. And don't forget your Signed-off-by line.
Please take a look at other patches on the list how this should be done. Best would be if you could use the git tools to create (and send) the patches.
Thanks.
Some further remarks below:
=================================================== diff -aupNr a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c --- a/drivers/mtd/cfi_flash.c 2008-04-21 02:39:38.000000000 +0400 +++ b/drivers/mtd/cfi_flash.c 2008-04-29 15:57:51.000000000 +0400 @@ -298,17 +298,29 @@ static inline void flash_unmap(flash_inf /*-----------------------------------------------------------------------
- make a proper sized command based on the port and chip widths
*/ -static void flash_make_cmd (flash_info_t * info, uchar cmd, void *cmdbuf) +static void flash_make_cmd (flash_info_t * info, ulong cmd, void *cmdbuf) { int i;
- int cpofft; uchar *cp = (uchar *) cmdbuf;
- uchar cp_val;
#if defined(__LITTLE_ENDIAN) || defined(CFG_WRITE_SWAPPED_DATA)
- for (i = info->portwidth; i > 0; i--)
- for (i = sizeof(cfiword_t); i > 0; i--)
- {
+ for (i = sizeof(cfiword_t); i > 0; i--) {
The code down below has some coding style issues too. Please
cpofft=(i-1);
+ cpofft = i - 1;
The code down below has some coding style issues too. Please try to be more careful here.
And the resulting code looks like this:
#if defined(__LITTLE_ENDIAN) || defined(CFG_WRITE_SWAPPED_DATA) for (i = sizeof(cfiword_t); i > 0; i--) { cpofft=(i-1); #else for (i = 1; i <= sizeof(cfiword_t); i++) { cpofft=(sizeof(cfiword_t)-i); #endif if( cpofft%info->chipwidth >= sizeof(ulong) || cpofft>=info->portwidth) cp_val = 0x00; else cp_val = *((uchar*)&cmd + cpofft%info->chipwidth); cp[i-1] = cp_val; }
Apart from the coding-style issues, this is getting quite complex and unclear. At least to me. Are you sure that this can't be written differently to make it easier to understand?
Thanks.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================