[U-Boot,RESEND,5/9] EEPROM: Add an EEPROM uclass

We might want to access data stored onto EEPROMs. Create a framework to
provide a consistent API.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/Kconfig                |  2 +-
 drivers/Makefile               |  1 +-
 drivers/eeprom/Kconfig         | 17 ++++++++++-
 drivers/eeprom/Makefile        |  2 +-
 drivers/eeprom/eeprom-uclass.c | 57 +++++++++++++++++++++++++++++++++++-
 include/dm/uclass-id.h         |  1 +-
 include/eeprom.h               | 21 +++++++++++++-
 7 files changed, 101 insertions(+), 0 deletions(-)
 create mode 100644 drivers/eeprom/Kconfig
 create mode 100644 drivers/eeprom/Makefile
 create mode 100644 drivers/eeprom/eeprom-uclass.c
 create mode 100644 include/eeprom.h

Hi Maxime,

On 8 November 2016 at 03:19, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> We might want to access data stored onto EEPROMs. Create a framework to
> provide a consistent API.

We have UCLASS_I2C_EEPROM. Can we unify these? If not, please add a
sandbox driver and test.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/Kconfig                |  2 +-
>  drivers/Makefile               |  1 +-
>  drivers/eeprom/Kconfig         | 17 ++++++++++-
>  drivers/eeprom/Makefile        |  2 +-
>  drivers/eeprom/eeprom-uclass.c | 57 +++++++++++++++++++++++++++++++++++-
>  include/dm/uclass-id.h         |  1 +-
>  include/eeprom.h               | 21 +++++++++++++-
>  7 files changed, 101 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/eeprom/Kconfig
>  create mode 100644 drivers/eeprom/Makefile
>  create mode 100644 drivers/eeprom/eeprom-uclass.c
>  create mode 100644 include/eeprom.h
>
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 74194b0d6f7f..e518752eae1a 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -20,6 +20,8 @@ source "drivers/dfu/Kconfig"
>
>  source "drivers/dma/Kconfig"
>
> +source "drivers/eeprom/Kconfig"
> +
>  source "drivers/fpga/Kconfig"
>
>  source "drivers/gpio/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 543c43bb0d23..c8f285f66bb3 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -64,6 +64,7 @@ obj-y += block/
>  obj-$(CONFIG_BOOTCOUNT_LIMIT) += bootcount/
>  obj-$(CONFIG_CPU) += cpu/
>  obj-y += crypto/
> +obj-y += eeprom/
>  obj-$(CONFIG_FPGA) += fpga/
>  obj-y += hwmon/
>  obj-y += misc/
> diff --git a/drivers/eeprom/Kconfig b/drivers/eeprom/Kconfig
> new file mode 100644
> index 000000000000..8dc597a8d894
> --- /dev/null
> +++ b/drivers/eeprom/Kconfig
> @@ -0,0 +1,17 @@
> +#
> +# EEPROM subsystem configuration
> +#
> +
> +menu "EEPROM support"
> +
> +config EEPROM
> +       bool "Enable EEPROM support"
> +       depends on DM
> +       help
> +         Support for the EEPROMs

Please expand this a bit.

> +
> +if EEPROM
> +
> +endif
> +
> +endmenu
> diff --git a/drivers/eeprom/Makefile b/drivers/eeprom/Makefile
> new file mode 100644
> index 000000000000..147dba5ec4b8
> --- /dev/null
> +++ b/drivers/eeprom/Makefile
> @@ -0,0 +1,2 @@
> +obj-$(CONFIG_EEPROM) += eeprom-uclass.o
> +
> diff --git a/drivers/eeprom/eeprom-uclass.c b/drivers/eeprom/eeprom-uclass.c
> new file mode 100644
> index 000000000000..020b0087d22c
> --- /dev/null
> +++ b/drivers/eeprom/eeprom-uclass.c
> @@ -0,0 +1,57 @@
> +/*
> + * Copyright (c) 2015 Free Electrons
> + * Copyright (c) 2015 NextThing Co.
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <eeprom.h>
> +
> +#include <dm/device-internal.h>
> +
> +int eeprom_read_buf(struct udevice *dev, unsigned offset,
> +                   u8 *buf, unsigned count)
> +{
> +       const struct eeprom_ops *ops = device_get_ops(dev);
> +
> +       if (!ops->read_buf)
> +               return -ENOSYS;
> +
> +       return ops->read_buf(dev, offset, buf, count);
> +}
> +
> +
> +UCLASS_DRIVER(eeprom) = {
> +       .name           = "eeprom",
> +       .id             = UCLASS_EEPROM,
> +};
> +
> +int eeprom_dm_init(void)
> +{
> +       struct udevice *bus;
> +       struct uclass *uc;
> +       int ret;
> +
> +       ret = uclass_get(UCLASS_EEPROM, &uc);
> +       if (ret)
> +               return ret;
> +
> +       uclass_foreach_dev(bus, uc) {
> +               ret = device_probe(bus);
> +               if (ret == -ENODEV) {   /* No such device. */
> +                       printf("EEPROM not available.\n");

debug()?

> +                       continue;
> +               }
> +
> +               if (ret) {              /* Other error. */
> +                       printf("EEPROM probe failed, error %d\n", ret);

debug()?

> +                       continue;
> +               }
> +       }
> +
> +       return 0;
> +}
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index b88adcbe802f..909a32404259 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -83,6 +83,7 @@ enum uclass_id {
>         UCLASS_VIDEO_BRIDGE,    /* Video bridge, e.g. DisplayPort to LVDS */
>         UCLASS_VIDEO_CONSOLE,   /* Text console driver for video device */
>         UCLASS_W1,              /* Dallas 1-Wire bus */
> +       UCLASS_EEPROM,          /* EEPROM */

Please put this in alpha order.

>
>         UCLASS_COUNT,
>         UCLASS_INVALID = -1,
> diff --git a/include/eeprom.h b/include/eeprom.h
> new file mode 100644
> index 000000000000..648c8606c35a
> --- /dev/null
> +++ b/include/eeprom.h
> @@ -0,0 +1,21 @@
> +/*
> + * Copyright (c) 2015 Free Electrons
> + * Copyright (c) 2015 NextThing Co
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#ifndef __EEPROM_H
> +#define __EEPROM_H
> +
> +#include <dm.h>

You can use a 'struct udevice *' forward decl instead.
> +
> +struct eeprom_ops {

Please add comments

> +       int     (*read_buf)(struct udevice *dev, unsigned offset,
> +                           u8 *buf, unsigned count);
> +};
> +
> +int eeprom_read_buf(struct udevice *dev, unsigned offset,
> +                   u8 *buf, unsigned count);
> +
> +#endif
> --
> git-series 0.8.11

Regards,
Simon

Hi Maxime,

On Fri, Nov 11, 2016 at 8:17 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Maxime,
>
> On 8 November 2016 at 03:19, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>> We might want to access data stored onto EEPROMs. Create a framework to
>> provide a consistent API.
>
> We have UCLASS_I2C_EEPROM. Can we unify these? If not, please add a
> sandbox driver and test.

I've been working on something very similar (the API looks the same obviously,
since the ops are pretty trivial, modulo function names)
In my opinion it should be working as follows:

UCLASS_EEPROM
  \... I2C_EEPROM
  \....SPI_EEPROM (AT25)

I also have some code to make the CMD_EEPROM stuff work, I'll submit a
follow up patch,
once we agreed on how the UCLASS_EEPROM looks like.

Cheers,

Moritz

Hi Moritz,

On Fri, Nov 11, 2016 at 11:13:39AM -0800, Moritz Fischer wrote:
> Hi Maxime,
> 
> On Fri, Nov 11, 2016 at 8:17 AM, Simon Glass <sjg@chromium.org> wrote:
> > Hi Maxime,
> >
> > On 8 November 2016 at 03:19, Maxime Ripard
> > <maxime.ripard@free-electrons.com> wrote:
> >> We might want to access data stored onto EEPROMs. Create a framework to
> >> provide a consistent API.
> >
> > We have UCLASS_I2C_EEPROM. Can we unify these? If not, please add a
> > sandbox driver and test.
> 
> I've been working on something very similar (the API looks the same obviously,
> since the ops are pretty trivial, modulo function names)
> In my opinion it should be working as follows:
> 
> UCLASS_EEPROM
>   \... I2C_EEPROM
>   \....SPI_EEPROM (AT25)

I agree. Different board variations might be using different buses to
store the same data, and it's all EEPROM anyway.

Maxime