[U-Boot] [PATCH v3 1/2] dm: implement a Miscellaneous uclass

[Simon Glass]

Hi Thomas,

On 29 September 2015 at 12:59, Thomas Chou <thomas at wytron.com.tw> wrote:
> Implement a Miscellaneous uclass with generic read or
> write operations. This class is used only for those
> do not fit other more general classes.
>
> Signed-off-by: Thomas Chou <thomas at wytron.com.tw>
> ---
> v2
>   add missing misc.h.
> v3
>   no change.
>
>  drivers/misc/Kconfig       |  6 ++++++
>  drivers/misc/Makefile      |  1 +
>  drivers/misc/misc-uclass.c | 14 ++++++++++++++
>  include/dm/uclass-id.h     |  1 +
>  include/misc.h             | 22 ++++++++++++++++++++++
>  5 files changed, 44 insertions(+)
>  create mode 100644 drivers/misc/misc-uclass.c
>  create mode 100644 include/misc.h
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 8b38a84..51c2a08 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -4,6 +4,12 @@
>
>  menu "Multifunction device drivers"
>
> +config DM_MISC
> +       bool "Enable Driver Model for Misc drivers"
> +       depends on DM
> +       help
> +         Enable driver model for miscellaneous devices.

Need to explain this a bit more! When do I use it, mention that it has
read and write.

I'm not sure about the name 'misc' but I suppose it makes sense. I
wonder if you should add 'ioctl' also?

> +
>  config CMD_CROS_EC
>         bool "Enable crosec command"
>         depends on CROS_EC
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 8d0fc3c..b285946 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -5,6 +5,7 @@
>  # SPDX-License-Identifier:     GPL-2.0+
>  #
>
> +obj-$(CONFIG_DM_MISC) += misc-uclass.o
>  obj-$(CONFIG_ALI152X) += ali512x.o
>  obj-$(CONFIG_DS4510)  += ds4510.o
>  obj-$(CONFIG_CBMEM_CONSOLE) += cbmem_console.o
> diff --git a/drivers/misc/misc-uclass.c b/drivers/misc/misc-uclass.c
> new file mode 100644
> index 0000000..40fe947
> --- /dev/null
> +++ b/drivers/misc/misc-uclass.c
> @@ -0,0 +1,14 @@
> +/*
> + * Copyright (C) 2010 Thomas Chou <thomas at wytron.com.tw>
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <misc.h>
> +
> +UCLASS_DRIVER(misc) = {
> +       .id             = UCLASS_MISC,
> +       .name           = "misc",
> +};
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index aff34a4..a6982ab 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -39,6 +39,7 @@ enum uclass_id {
>         UCLASS_LED,             /* Light-emitting diode (LED) */
>         UCLASS_LPC,             /* x86 'low pin count' interface */
>         UCLASS_MASS_STORAGE,    /* Mass storage device */
> +       UCLASS_MISC,            /* Miscellaneous device */
>         UCLASS_MMC,             /* SD / MMC card or chip */
>         UCLASS_MOD_EXP,         /* RSA Mod Exp device */
>         UCLASS_PCH,             /* x86 platform controller hub */
> diff --git a/include/misc.h b/include/misc.h
> new file mode 100644
> index 0000000..dfca41f
> --- /dev/null
> +++ b/include/misc.h
> @@ -0,0 +1,22 @@
> +/*
> + * Copyright (C) 2015 Thomas Chou <thomas at wytron.com.tw>
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#ifndef _DM_MISC_H_
> +#define _DM_MISC_H_
> +
> +/*
> + * struct dm_misc_ops - Driver model Misc operations
> + *
> + * The uclass interface is implemented by all miscellaneous devices which
> + * use driver model.
> + */
> +struct dm_misc_ops {
> +       int (*read)(struct udevice *dev, int offset, void *buf, int size);
> +       int (*write)(struct udevice *dev, int offset, const void *buf,
> +                    int size);

Method comments
> +};

Also don't you need to create misc_read() and misc_write() functions
so people can use it?

> +
> +#endif /* _DM_MISC_H_ */
> --
> 2.1.4
>

Regards,
Simon