Messages in this thread | | | From | Jingoo Han <> | Subject | Re: [PATCH v5] mtd:nor:timeout:fix do_write_buffer() timeout error | Date | Wed, 16 Jul 2014 19:22:27 +0900 |
| |
On Wednesday, July 16, 2014 5:49 PM, Bean Huo wrote: > > >> + if (cfi->cfiq->BufWriteTimeoutTyp && > >> + cfi->cfiq->BufWriteTimeoutMax){ > >> + cfi->chips[i].buffer_write_time_max = > >> + 1<<(cfi->cfiq->BufWriteTimeoutTyp + > >> + cfi->cfiq->BufWriteTimeoutMax); > >> + } else { > >> + cfi->chips[i].buffer_write_time_max = 0; > >> + } > > > > Please keep the coding style as below. > > > > if ( ) { > > ... > > } else { > > ... > > } > > > > If I keep coding style as above.this will be beyond the requirements of one line length. > I also saw othter files use the same code style as mine.please see:cfi_cmdset_0001.c > > if (cfi->cfiq->WordWriteTimeoutTyp && > cfi->cfiq->WordWriteTimeoutMax) > cfi->chips[i].word_write_time_max = > 1<<(cfi->cfiq->WordWriteTimeoutTyp + > cfi->cfiq->WordWriteTimeoutMax); > else > cfi->chips[i].word_write_time_max = 50000 * 8;
I don't want to mention about braces. However, I said that you should keep the indentation.
Your original patch
+ if (cfi->cfiq->BufWriteTimeoutTyp && + cfi->cfiq->BufWriteTimeoutMax){ + cfi->chips[i].buffer_write_time_max = + 1<<(cfi->cfiq->BufWriteTimeoutTyp + + cfi->cfiq->BufWriteTimeoutMax); + } else { + cfi->chips[i].buffer_write_time_max = 0; + } + cfi->chips[i].buffer_write_time_max = + ((cfi->chips[i].buffer_write_time_max>= 2000) + ? cfi->chips[i].buffer_write_time_max : 2000);
My suggestion is as follows:
+ if (cfi->cfiq->BufWriteTimeoutTyp && + cfi->cfiq->BufWriteTimeoutMax){ + cfi->chips[i].buffer_write_time_max = + 1<<(cfi->cfiq->BufWriteTimeoutTyp + + cfi->cfiq->BufWriteTimeoutMax); + } else { + cfi->chips[i].buffer_write_time_max = 0; + } + cfi->chips[i].buffer_write_time_max = + ((cfi->chips[i].buffer_write_time_max>= 2000) + ? cfi->chips[i].buffer_write_time_max : 2000);
Braces? I have no idea. Actually, I prefer to use braces for the readability. However, according to the Document/CodingStyle, these braces are unnecessary.
However, one thing is clear; 'if' and 'else' should use the same indentation level as below.
1. if(){ ... } else { ... }
2. if(){ ... } else { ... }
3.
if(){ ... } else { ... }
Best regards, Jingoo Han
> > Note: beacuse I cann't send mail by git in office,I send this patch by web mail client.So, > there still a error stype in this patch(web mail client will remove some spaces),that is one space > required before that '>='.I will send again this patch at home. > For your coding style advice, I will take into account,but i don't know if can put one line. > =
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |