[U-Boot] [PATCH v9 01/21] dm: mtd: Add dm mtd core ops

[Simon Glass]

Hi Jagan,

On 30 October 2016 at 12:23, Jagan Teki <jagan at openedev.com> wrote:
> - Add generic dm_mtd operations
> - Add mtd_read|erase|write_dm
> - Add add_mtd_device_dm
>
> The respetive MTD_UCLASS drivers must install the hooks to these
> dm_mtd_ops and other core ops are act as a interface b/w drivers
> vs command code.
>
> Signed-off-by: Jagan Teki <jagan at openedev.com>
> ---
>  drivers/mtd/mtd-uclass.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/mtd.h            | 54 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 126 insertions(+)

Reviewed-by: Simon Glass <sjg at chromium.org>

But please see changes below.

>
> diff --git a/drivers/mtd/mtd-uclass.c b/drivers/mtd/mtd-uclass.c
> index 7b7c48e..acbd713 100644
> --- a/drivers/mtd/mtd-uclass.c
> +++ b/drivers/mtd/mtd-uclass.c
> @@ -1,4 +1,5 @@
>  /*
> + * Copyright (C) 2016 Jagan Teki <jagan at openedev.com>
>   * Copyright (C) 2015 Thomas Chou <thomas at wytron.com.tw>
>   *
>   * SPDX-License-Identifier:    GPL-2.0+
> @@ -8,6 +9,77 @@
>  #include <dm.h>
>  #include <errno.h>
>  #include <mtd.h>
> +#include <linux/log2.h>
> +
> +int dm_mtd_read(struct udevice *dev, loff_t from, size_t len, size_t *retlen,
> +               u_char *buf)
> +{
> +       struct mtd_info *mtd = mtd_get_info(dev);
> +
> +       *retlen = 0;
> +       if (from < 0 || from > mtd->size || len > mtd->size - from)
> +               return -EINVAL;
> +       if (!len)
> +               return 0;
> +
> +       return mtd_get_ops(dev)->_read(dev, from, len, retlen, buf);
> +}
> +
> +int dm_mtd_erase(struct udevice *dev, struct erase_info *instr)
> +{
> +       struct mtd_info *mtd = mtd_get_info(dev);
> +
> +       if (instr->addr > mtd->size || instr->len > mtd->size - instr->addr)
> +               return -EINVAL;
> +       if (!(mtd->flags & MTD_WRITEABLE))
> +               return -EROFS;
> +       instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
> +       if (!instr->len) {
> +               instr->state = MTD_ERASE_DONE;
> +               return 0;
> +       }
> +
> +       return mtd_get_ops(dev)->_erase(dev, instr);
> +}
> +
> +int dm_mtd_write(struct udevice *dev, loff_t to, size_t len, size_t *retlen,
> +                const u_char *buf)
> +{
> +       struct mtd_info *mtd = mtd_get_info(dev);
> +
> +       *retlen = 0;
> +       if (to < 0 || to > mtd->size || len > mtd->size - to)
> +               return -EINVAL;
> +       if (!mtd_get_ops(dev)->_write || !(mtd->flags & MTD_WRITEABLE))
> +               return -EROFS;
> +       if (!len)
> +               return 0;
> +
> +       return mtd_get_ops(dev)->_write(dev, to, len, retlen, buf);
> +}
> +
> +int dm_add_mtd_device(struct udevice *dev)
> +{
> +       struct mtd_info *mtd = mtd_get_info(dev);
> +
> +       BUG_ON(mtd->writesize == 0);
> +       mtd->usecount = 0;
> +
> +       if (is_power_of_2(mtd->erasesize))
> +               mtd->erasesize_shift = ffs(mtd->erasesize) - 1;
> +       else
> +               mtd->erasesize_shift = 0;
> +
> +       if (is_power_of_2(mtd->writesize))
> +               mtd->writesize_shift = ffs(mtd->writesize) - 1;
> +       else
> +               mtd->writesize_shift = 0;
> +
> +       mtd->erasesize_mask = (1 << mtd->erasesize_shift) - 1;
> +       mtd->writesize_mask = (1 << mtd->writesize_shift) - 1;
> +
> +       return 0;
> +}
>
>  /*
>   * Implement a MTD uclass which should include most flash drivers.
> diff --git a/include/mtd.h b/include/mtd.h
> index 3f8c293..93b5eaf 100644
> --- a/include/mtd.h
> +++ b/include/mtd.h
> @@ -20,4 +20,58 @@ static inline struct mtd_info *mtd_get_info(struct udevice *dev)
>         return dev_get_uclass_priv(dev);
>  }
>
> +struct dm_mtd_ops {
> +       int (*_erase)(struct udevice *dev, struct erase_info *instr);
> +       int (*_read)(struct udevice *dev, loff_t from, size_t len,
> +                    size_t *retlen, u_char *buf);
> +       int (*_write)(struct udevice *dev, loff_t to, size_t len,
> +                     size_t *retlen, const u_char *buf);

Please add detailed comments to the struct and each method. Also can
you drop the underscore?

> +};
> +
> +/* Access the serial operations for a device */
> +#define mtd_get_ops(dev) ((struct dm_mtd_ops *)(dev)->driver->ops)
> +
> +/**
> + * dm_mtd_read() - Read data from MTD device
> + *
> + * @dev:       MTD device
> + * @from:      Offset into device in bytes to read from
> + * @len:       Length of bytes to read
> + * @retlen:    Length of return bytes read to
> + * @buf:       Buffer to put the data that is read
> + * @return 0 if OK, -ve on error
> + */
> +int dm_mtd_read(struct udevice *dev, loff_t from, size_t len, size_t *retlen,
> +               u_char *buf);
> +
> +/**
> + * dm_mtd_write() - Write data to MTD device
> + *
> + * @dev:       MTD device
> + * @to:                Offset into device in bytes to write to
> + * @len:       Length of bytes to write
> + * @retlen:    Length of return bytes to write to
> + * @buf:       Buffer containing bytes to write
> + * @return 0 if OK, -ve on error
> + */
> +int dm_mtd_write(struct udevice *dev, loff_t to, size_t len, size_t *retlen,
> +                const u_char *buf);
> +
> +/**
> + * dm_mtd_erase() - Erase blocks of the MTD device
> + *
> + * @dev:       MTD device
> + * @instr:     Erase info details of MTD device
> + * @return 0 if OK, -ve on error
> + */
> +int dm_mtd_erase(struct udevice *dev, struct erase_info *instr);
> +
> +/**
> + * dm_add_mtd_device() - Add MTD device
> + *
> + * @dev:       MTD device
> + * @return 0 if OK, -ve on error
> + */
> +int dm_add_mtd_device(struct udevice *dev);
> +
>  #endif /* _MTD_H_ */
> --
> 2.7.4
>

Regards,
Simon