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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
