On 10/31/2011 08:23 AM, Marek Vasut wrote:
> +inline uint16_t onenand_readw(uint32_t addr)
> +{
> +     return readw(CONFIG_SYS_ONENAND_BASE + addr);
> +}
> +
> +inline void onenand_writew(uint16_t value, uint32_t addr)
> +{
> +     writew(value, CONFIG_SYS_ONENAND_BASE + addr);
> +}

static

> +#define      onenand_block_address(block)            (block)
> +#define      onenand_sector_address(page)            (page << 2)
> +#define      onenand_buffer_address()                ((1 << 3) << 8)
> +#define      onenand_bufferram_address(block)        (0)

Space rather than tab after #define

> +void spl_onenand_get_geometry(struct spl_onenand_data *data)
> +{
> +     uint32_t tmp;
> +     uint32_t dev_id, density;
> +
> +     /* Default geometry -- 2048b page, 128k erase block. */
> +     data->pagesize = 2048;
> +     data->erasesize = 0x20000;
> +
> +     tmp = onenand_readw(ONENAND_REG_TECHNOLOGY);
> +     if (tmp)
> +             goto dev_4k;
> +
> +     dev_id = onenand_readw(ONENAND_REG_DEVICE_ID);
> +     density = dev_id >> ONENAND_DEVICE_DENSITY_SHIFT;
> +     density &= ONENAND_DEVICE_DENSITY_MASK;
> +
> +     if (density < ONENAND_DEVICE_DENSITY_4Gb)
> +             return;
> +
> +     if (dev_id & ONENAND_DEVICE_IS_DDP)
> +             return;
> +
> +     /* 4k device geometry -- 4096b page, 256k erase block. */
> +dev_4k:
> +     data->pagesize = 4096;
> +     data->erasesize = 0x40000;
> +}

This seems like a gratuitous use of goto...

> +int spl_onenand_read_block(uint32_t block, uint8_t *buf, uint32_t *read)
> +{
> +     struct spl_onenand_data data;
> +     uint32_t page;
> +     int ret;
> +
> +     spl_onenand_get_geometry(&data);
> +
> +     for (page = 0; page < ONENAND_PAGES_PER_BLOCK; page++) {
> +             ret = spl_onenand_read_page(block, page, buf, data.pagesize);
> +             if (ret)
> +                     return ret;
> +             buf += data.pagesize;
> +     }

Shouldn't this do bad block skipping rather than error on the first bad
block it sees?  The current onenand IPL does this.

> +     *read = ((block * ONENAND_PAGES_PER_BLOCK) + page) * data.pagesize;

We only read one block here, but we return the byte address of the next
block?  A little odd, and if it's really what's intended, needs to be
documented.

> diff --git a/include/onenand_uboot.h b/include/onenand_uboot.h
> index 92279d5..66828ce 100644
> --- a/include/onenand_uboot.h
> +++ b/include/onenand_uboot.h
> @@ -16,6 +16,8 @@
>  
>  #include <linux/types.h>
>  
> +#ifndef      CONFIG_SPL_BUILD
> +
>  /* Forward declarations */
>  struct mtd_info;
>  struct mtd_oob_ops;
> @@ -52,4 +54,20 @@ 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
> +
> +#define ONENAND_PAGES_PER_BLOCK              64
> +
> +struct spl_onenand_data {
> +     uint32_t        pagesize;
> +     uint32_t        erasesize;
> +};
> +
> +void spl_onenand_get_geometry(struct spl_onenand_data *data);
> +int spl_onenand_read_page(uint32_t block, uint32_t page,
> +                             uint8_t *buf, int pagesize);
> +int spl_onenand_read_block(uint32_t block, uint8_t *buf, uint32_t *read);
> +
> +#endif

Do these really need to be #ifdeffed?

-Scott

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to