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

Singh, Vimal
2009-06-26T15:45:23Z

On Fri, Jun 26, 2009 at 8:39 PM, David Woodhouse<dwmw2@infradead.org> wrote:
> On Fri, 2009-06-26 at 21:21 +0800, tom.leiming@gmail.com wrote:
>> From: Ming Lei <tom.leiming@gmail.com>
>>
>> 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@gmail.com>
>> ---
>> 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@intel.com Intel Corporation
>
> --
> 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/

---
Vimal