[U-Boot] [PATCH 4/9] drivers: nand: implement a NAND uclass

Scott Wood
2016-04-01T23:25:39Z

On Fri, 2016-04-01 at 16:59 +0530, Mugunthan V N wrote:
> +static int nand_child_pre_probe(struct udevice *dev)
> +{
> + nand_info_t *nand = dev_get_uclass_priv(dev);
> + void *priv = dev_get_priv(dev);
> +
> + /*
> + * Store nand device priv pointer in nand_info so that
> + * it can be used by nand command
> + */
> + nand->priv = priv;

Wouldn't it make more sense to have a pointer to the device in the NAND
struct, and let the driver manage both privs as it chooses?

> +
> + return 0;
> +}
> +
> +UCLASS_DRIVER(nand) = {
> + .id = UCLASS_NAND,
> + .name = "nand",
> + .flags = DM_UC_FLAG_SEQ_ALIAS,
> + .post_probe = nand_child_pre_probe,
> + .per_device_auto_alloc_size = sizeof(nand_info_t),
> +};

Post probe = pre probe?

> @@ -63,8 +63,10 @@ int nand_register(int devnum)
> static void nand_init_chip(int i)
> {
> struct mtd_info *mtd;
> +#ifndef CONFIG_DM_NAND
> struct nand_chip *nand = &nand_chip[i];
> ulong base_addr = base_address[i];
> +#endif
> int maxchips = CONFIG_SYS_NAND_MAX_CHIPS;
>
> if (maxchips < 1)
> @@ -74,11 +76,13 @@ static void nand_init_chip(int i)
> if (!mtd)
> return;
>
> +#ifndef CONFIG_DM_NAND
> mtd->priv = nand;
> nand->IO_ADDR_R = nand->IO_ADDR_W = (void __iomem *)base_addr;
>
> if (board_nand_init(nand))
> return;
> +#endif
>
> if (nand_scan(mtd, maxchips))
> return;

Please do not use this code for DM. Have drivers' probe functions call
nand_scan (split into ident and tail if necessary) as is done with
CONFIG_SYS_NAND_SELF_INIT.

> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 37c4176..6a88d39 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -49,6 +49,7 @@ enum uclass_id {
> UCLASS_MMC, /* SD / MMC card or chip */
> UCLASS_MOD_EXP, /* RSA Mod Exp device */
> UCLASS_MTD, /* Memory Technology Device (MTD) device
> */
> + UCLASS_NAND, /* NAND bus */

NAND is not a bus.

> +#ifndef CONFIG_DM_NAND
> static inline nand_info_t *get_nand_dev_by_index(int dev)
> {
> if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE ||
> @@ -154,5 +156,8 @@ static inline nand_info_t *get_nand_dev_by_index(int
> dev)
>
> return &nand_info[dev];
> }
> +#else
> +nand_info_t *get_nand_dev_by_index(int idx);
> +#endif

Please use "if X/else", not "if !X/else".

-Scott