> On 11/01/2011 05:54 PM, Marek Vasut wrote:
> > +static void spl_onenand_get_geometry(struct spl_onenand_data *data)
> > +{
[...]
> > + /* The page can be either 2k or 4k, avoid using DIV_ROUND_UP. */
> > + if (data.pagesize == 2048) {
> > + total_pages = len / 2048;
> > + page = offset / 2048;
> > + total_pages += !!(len & 2047);
> > + } else if (data.pagesize == 4096) {
> > + total_pages = len / 4096;
> > + page = offset / 4096;
> > + total_pages += !!(len & 4095);
> > + }
>
> What's wrong with DIV_ROUND_UP? It should produce smaller code than
> what you've done here...
It pulls in aeabi_*div* functions, which won't fit into block 0 of Onenand.
>
> > + for (; page <= total_pages; page++) {
> > + block = page / ONENAND_PAGES_PER_BLOCK;
> > + rpage = page & (ONENAND_PAGES_PER_BLOCK - 1);
> > + ret = spl_onenand_read_page(block, rpage, addr, data.pagesize);
> > + if (ret) {
> > + total_pages++;
> > + err |= 1;
> > + } else
> > + addr += data.pagesize / 4;
> > + }
>
> As discussed, please retain the existing block-skipping semantics. And
> if you do skip a block, that's not an error to be propagated upward (as
> opposed to something like an uncorrectable ECC error).
>
> If one side of an if statement requires braces, both sides should have
> them.
>
> > diff --git a/include/onenand_uboot.h b/include/onenand_uboot.h
> > index 92279d5..fcb50ff 100644
> > --- a/include/onenand_uboot.h
> > +++ b/include/onenand_uboot.h
> > @@ -16,6 +16,8 @@
> >
> > #include <linux/types.h>
> >
> > +#ifndef CONFIG_SPL_BUILD
> > +
>
> Please use a space rather than a tab after #define, #ifndef, etc.
>
> > /* Forward declarations */
> > struct mtd_info;
> > struct mtd_oob_ops;
> >
> > @@ -52,4 +54,10 @@ extern int flexonenand_set_boundary(struct mtd_info
> > *mtd, int die,
> >
> > extern void s3c64xx_onenand_init(struct mtd_info *);
> > extern void s3c64xx_set_width_regs(struct onenand_chip *);
> >
> > +#else
> > +
> > +int spl_onenand_load_image(uint32_t dst, uint32_t offset, uint32_t len);
> > +
> > +#endif
>
> Why does this need to be ifdeffed at all? We normally don't ifdef
> header declarations.
>
> -Scott
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
