On Sat, 2015-07-18 at 01:47 +0300, Vladimir Zapolskiy wrote:
> +/* TAC register bits, be aware of overflows */
> +#define TAC_W_RDY(n) (max_t(uint32_t, (n), 0xF) << 28)
> +#define TAC_W_WIDTH(n) (max_t(uint32_t, (n), 0xF) << 24)
> +#define TAC_W_HOLD(n) (max_t(uint32_t, (n), 0xF) << 20)
> +#define TAC_W_SETUP(n) (max_t(uint32_t, (n), 0xF) << 16)
> +#define TAC_R_RDY(n) (max_t(uint32_t, (n), 0xF) << 12)
> +#define TAC_R_WIDTH(n) (max_t(uint32_t, (n), 0xF) << 8)
> +#define TAC_R_HOLD(n) (max_t(uint32_t, (n), 0xF) << 4)
> +#define TAC_R_SETUP(n) (max_t(uint32_t, (n), 0xF) << 0)
> +
> +static struct lpc32xx_nand_slc_registers __iomem
> *lpc32xx_nand_slc_registers
> + = (struct lpc32xx_nand_slc_registers __iomem *)SLC_NAND_BASE;
s/registers/regs/ might help the formatting here...
> +static void lpc32xx_nand_cmd_ctrl(struct mtd_info *mtd,
> + int cmd, unsigned int ctrl)
> +{
> + debug("ctrl: 0x%08x, cmd: 0x%08x\n", ctrl, cmd);
> +
> + if (ctrl & NAND_NCE)
> + setbits_le32(&lpc32xx_nand_slc_registers->cfg, CFG_CE_LOW);
> + else
> + clrbits_le32(&lpc32xx_nand_slc_registers->cfg, CFG_CE_LOW);
> +
> + if (cmd == NAND_CMD_NONE)
> + return;
> +
> + if (ctrl & NAND_CLE)
> + writel(cmd & 0xFF, &lpc32xx_nand_slc_registers->cmd);
> + else /* if (ctrl & NAND_ALE) */
> + writel(cmd & 0xFF, &lpc32xx_nand_slc_registers->addr);
> +}
Why is "if (ctrl & NAND_ALE())" commented out? I think it'd be better if
wrong uses of this function didn't silently cause address cycles (which could
hide a bug if ALE is what was desired).
> +static void lpc32xx_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
> +{
> + while (len-- > 0)
> + *buf++ = (uint8_t)readl(&lpc32xx_nand_slc_registers->data);
> +}
> +
> +static uint8_t lpc32xx_read_byte(struct mtd_info *mtd)
> +{
> + return (uint8_t)readl(&lpc32xx_nand_slc_registers->data);
> +}
You've still got a couple unneeded casts here.
> +/*
> + * LPC32xx has only one SLC NAND controller, don't utilize
> + * CONFIG_SYS_NAND_SELF_INIT to be able to reuse this function
> + * both in SPL NAND and U-boot images.
> + */
> +int board_nand_init(struct nand_chip *lpc32xx_chip)
> +{
> + lpc32xx_chip->cmd_ctrl = lpc32xx_nand_cmd_ctrl;
> + lpc32xx_chip->dev_ready = lpc32xx_nand_dev_ready;
> +
> + /*
> + * Hardware ECC calculation is not supported by the driver,
> + * because it requires DMA support, see LPC32x0 User Manual,
> + * note after SLC_ECC register description (UM10326, p.198)
> + */
> + lpc32xx_chip->ecc.mode = NAND_ECC_SOFT;
> +
> + /*
> + * The implementation of these functions is quite common, but
> + * they MUST be defined, because access to data register
> + * is strictly 32-bit aligned.
> + */
> + lpc32xx_chip->read_buf = lpc32xx_read_buf;
> + lpc32xx_chip->read_byte = lpc32xx_read_byte;
> + lpc32xx_chip->write_buf = lpc32xx_write_buf;
> + lpc32xx_chip->write_byte = lpc32xx_write_byte;
> +
> + /*
> + * Use default ECC layout, but these values are predefined
> + * for both small and large page NAND flash devices.
> + */
> + lpc32xx_chip->ecc.size = 256;
> + lpc32xx_chip->ecc.bytes = 3;
> + lpc32xx_chip->ecc.strength = 1;
Please use a space before '=', not a tab. This doesn't even line up right,
if that's what you were trying to do...
> +
> +#if defined(CONFIG_SYS_NAND_USE_FLASH_BBT)
> + lpc32xx_chip->bbt_options |= NAND_BBT_USE_FLASH;
> +#endif
Likewise
-Scott
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
