RE: [PATCH][MTD][NAND]: fix omap2.c compile failure and warning

[Singh, Vimal]

On Fri, Jun 26, 2009 at 8:39 PM, David Woodhouse<dwmw2@xxxxxxxxxxxxx> wrote:
> On Fri, 2009-06-26 at 21:21 +0800, tom.leiming@xxxxxxxxx wrote:
>> From: Ming Lei <tom.leiming@xxxxxxxxx>
>>
>> drivers/mtd/nand/omap2.c: In function 'omap_wait':
>> drivers/mtd/nand/omap2.c:543: error: 'jiffies' undeclared (first use in this function)
>> drivers/mtd/nand/omap2.c:543: error: (Each undeclared identifier is reported only once
>> drivers/mtd/nand/omap2.c:543: error: for each function it appears in.)
>> drivers/mtd/nand/omap2.c:557: error: implicit declaration of function 'time_before'
>
> OK.
>
>> ......
>>
>> drivers/mtd/nand/omap2.c: In function 'omap_wait':
>> drivers/mtd/nand/omap2.c:545: warning: 'status' may be used uninitialized in this function
>
> Hm, why do you set it to zero? In the (fairly unlikely) case where you
> never go through the loop at all, because you're preempted until the
> timeout has completed, what value do you want it to return?
>
> Don't just make changes by bashing on the keyboard until the compiler
> shuts up; please apply brain.
>
> That way, I think you'll notice that the return value from this function
> is completely screwed anyway.... Vimal? Shouldn't it be returning
> NAND_STATUS_FAIL in the failure case, not a value read directly from the
> hardware register? And shouldn't there be a cpu_relax() (or even
> schedule) in there too?

That's correct David. I think 'cond_resched()' would be fine here.

>
>> Signed-off-by: Ming Lei <tom.leiming@xxxxxxxxx>
>> ---
>>  drivers/mtd/nand/omap2.c |    4 +++-
>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
>> index 0cd76f8..2905c79 100644
>> --- a/drivers/mtd/nand/omap2.c
>> +++ b/drivers/mtd/nand/omap2.c
>> @@ -15,6 +15,7 @@
>>  #include <linux/mtd/nand.h>
>>  #include <linux/mtd/partitions.h>
>>  #include <linux/io.h>
>> +#include <linux/jiffies.h>
>>
>>  #include <asm/dma.h>
>>
>> @@ -541,7 +542,8 @@ static int omap_wait(struct mtd_info *mtd, struct nand_chip *chip)
>>       struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
>>                                                       mtd);
>>       unsigned long timeo = jiffies;
>> -     int status, state = this->state;
>> +     int state = this->state;
>> +     int status = 0;
>>
>>       if (state == FL_ERASING)
>>               timeo += (HZ * 400) / 1000;
> --
> David Woodhouse                            Open Source Technology Centre
> David.Woodhouse@xxxxxxxxx                              Intel Corporation
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

---
Vimal--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/