Hi Rohit / Moorthy,
Do we have coding guidelines from open source? Let us follow that. It is very good that they are reviewing in depth this also means that they are much interested in our products.
Regards, Prakash Talawar
----- Original Message ----- From: "Scott Wood" scottwood@freescale.com To: "Rohit Hagargundgi" h.rohit@samsung.com Cc: u-boot@lists.denx.de; k.rajesh@samsung.com; prakash.t@samsung.com; amitsharma.9@samsung.com Sent: Friday, January 16, 2009 2:42 AM Subject: Re: [U-Boot] [PATCH 1/3] Bad block support for environment variables partition in OneNAND
On Thu, Dec 11, 2008 at 07:58:26PM +0530, Rohit Hagargundgi wrote:
- env_addr = FLEXONENAND(this) ? CONFIG_ENV_ADDR_FLEX :
- CONFIG_ENV_ADDR;
- env_len = FLEXONENAND(this) ? CONFIG_ENV_SIZE_FLEX :
- CONFIG_ENV_SIZE;
Is there any reason why this has to be done at runtime, rather than just using one set of CONFIG_ parameters regardless of whether it's flex?
/* Check OneNAND exist */
- if (onenand_mtd.writesize)
- /* Ignore read fail */
- onenand_read(&onenand_mtd, env_addr, onenand_mtd.writesize,
&retlen, (u_char *) env_ptr);- else
- if (onenand_mtd.writesize) {
- while (env_addr < env_addr + env_len) {
This is a tautology, unless you're checking for wraparound of unsigned long.
- onenand_get_block(this, ofs, &slc);
- if (slc)
- instr.len += 1 << (this->erase_shift -1);
this->erase_shift - 1
- if (mtd->erase(mtd, &instr)) {
- printf("writeenv: erase failed at 0x%08lx\n", ofs);
- if(mtd->block_markbad(mtd, ofs))
- printf("writeenv: Mark bad failed at 0x%08lx\n", ofs);
Space after "if".
-Scott