> -----Original Message----- > From: Vimal Singh [mailto:vimal.newwork@xxxxxxxxx] > Sent: 2010-05-17 19:57 > To: Ghorai, Sukumar > Cc: linux-omap@xxxxxxxxxxxxxxx; Artem.Bityutskiy@xxxxxxxxx; > tony@xxxxxxxxxxx; sakoman@xxxxxxxxx; linux-mtd@xxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH v2 1/2] omap3 nand: cleanup for not to use GPMC > virtual address > > On Mon, May 17, 2010 at 9:52 AM, Ghorai, Sukumar <s-ghorai@xxxxxx> wrote: > [...] > >> > @@ -108,11 +108,27 @@ static u32 gpmc_read_reg(int idx) > >> > return __raw_readl(gpmc_base + idx); > >> > } > >> > > >> > +static void gpmc_cs_write_byte(int cs, int idx, u8 val) > >> > +{ > >> > + void __iomem *reg_addr; > >> > + > >> > + reg_addr = gpmc_base + GPMC_CS0_BASE + (cs * GPMC_CS_SIZE) + > >> idx; > >> > + __raw_writeb(val, reg_addr); > >> > +} > >> > + > >> > +static u8 gpmc_cs_read_byte(int cs, int idx) > >> > +{ > >> > + void __iomem *reg_addr; > >> > + > >> > + reg_addr = gpmc_base + GPMC_CS0_BASE + (cs * GPMC_CS_SIZE) + > >> idx; > >> > + return __raw_readb(reg_addr); > >> > +} > >> > + > >> > >> I do not think we need these functions. > > [Ghorai] This is used in gpmc_hwcontrol() and to get the nand status > from omap2.c. > > Yes, I can see that. But I think you should read complete register > (32-bits) and the manipulate them accordingly. > > > > [...] > >> > @@ -229,14 +191,15 @@ static void omap_read_buf8(struct mtd_info > *mtd, > >> u_char *buf, int len) > >> > */ > >> > static void omap_write_buf8(struct mtd_info *mtd, const u_char *buf, > >> int len) > >> > { > >> > - struct omap_nand_info *info = container_of(mtd, > >> > - struct > omap_nand_info, > >> mtd); > >> > + u32 status; > >> > + struct nand_chip *nand = mtd->priv; > >> > u_char *p = (u_char *)buf; > >> > > >> > while (len--) { > >> > - iowrite8(*p++, info->nand.IO_ADDR_W); > >> > - while (GPMC_BUF_EMPTY == (readl(info->gpmc_baseaddr + > >> > - GPMC_STATUS) & > >> GPMC_BUF_FULL)); > >> > + iowrite8(*p++, nand->IO_ADDR_W); > >> > + gpmc_hwcontrol(0, 0, GPMC_GET_SET_STATUS, 0, > &status); > >> If I am not mistaking, 2nd argument is 'cs', correct? And then, why > >> are you hard coding this? > >> Different boards will have NAND chip present at different 'cs'. > >> Please have a look at uses of 'gpmc_hwcontrol' elsewhere as well for > this. > > [Ghorai] I agree. > >> > >> Again, say, you got '(status & GPMC_BUF_FULL) != GPMC_BUF_EMPTY', then: > >> > >> > + while (GPMC_BUF_EMPTY == (status & GPMC_BUF_FULL)) > >> > + ; > >> > >> You got in an infinite loop here? > > [Ghorai] if you see carefully this is same as existing code. Let me > check if any better solution. > > No. Look carefully. In previous code 'gpmc status' was being read in > each loop, while in your code you read it once and then you never look > for updated value. > That's why your code is going into infinite loop [Ghorai] ok. thanks > > > -- > Regards, > Vimal Singh -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
