Re: [PATCHv5 6/7] eeprom: 93xx46: extend driver to plug into the NVMEM framework

[Joachim Eastwood]

Hi Andrew,

On 17 February 2016 at 21:07, Andrew Lunn <andrew@xxxxxxx> wrote:
> Add a regmap for accessing the EEPROM, and then use that with the
> NVMEM framework. Enable backward compatibility in the MVMEM config

typo: MVMEM

> structure, so that the 'eeprom' file in sys is provided by the
> framework.
>
> Signed-off-by: Andrew Lunn <andrew@xxxxxxx>
> Acked-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
> ---
> v5: Remove useless test.
> ---
>  drivers/misc/eeprom/Kconfig         |   2 +
>  drivers/misc/eeprom/eeprom_93xx46.c | 121 ++++++++++++++++++++++++++++--------
>  2 files changed, 96 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig
> index 8c43a222ae55..cfc493c2e30a 100644
> --- a/drivers/misc/eeprom/Kconfig
> +++ b/drivers/misc/eeprom/Kconfig
> @@ -78,6 +78,8 @@ config EEPROM_93CX6
>  config EEPROM_93XX46
>         tristate "Microwire EEPROM 93XX46 support"
>         depends on SPI && SYSFS
> +       select REGMAP
> +       select NVMEM
>         help
>           Driver for the microwire EEPROM chipsets 93xx46x. The driver
>           supports both read and write commands and also the command to
> diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
> index ff63f05edc76..4dd9b5100f63 100644
> --- a/drivers/misc/eeprom/eeprom_93xx46.c
> +++ b/drivers/misc/eeprom/eeprom_93xx46.c
> @@ -15,7 +15,8 @@
>  #include <linux/mutex.h>
>  #include <linux/slab.h>
>  #include <linux/spi/spi.h>
> -#include <linux/sysfs.h>
> +#include <linux/nvmem-provider.h>
> +#include <linux/regmap.h>
>  #include <linux/eeprom_93xx46.h>
>
>  #define OP_START       0x4
> @@ -28,25 +29,29 @@
>  struct eeprom_93xx46_dev {
>         struct spi_device *spi;
>         struct eeprom_93xx46_platform_data *pdata;
> -       struct bin_attribute bin;
>         struct mutex lock;
> +       struct regmap_config regmap_config;
> +       struct nvmem_config nvmem_config;
> +       struct nvmem_device *nvmem;
>         int addrlen;
> +       int size;
>  };
>
>  static ssize_t
> -eeprom_93xx46_bin_read(struct file *filp, struct kobject *kobj,
> -                      struct bin_attribute *bin_attr,
> -                      char *buf, loff_t off, size_t count)
> +eeprom_93xx46_read(struct eeprom_93xx46_dev *edev, char *buf,
> +                  unsigned off, size_t count)
>  {
> -       struct eeprom_93xx46_dev *edev;
> -       struct device *dev;
>         struct spi_message m;
>         struct spi_transfer t[2];
>         int bits, ret;
>         u16 cmd_addr;
>
> -       dev = container_of(kobj, struct device, kobj);
> -       edev = dev_get_drvdata(dev);
> +       if (unlikely(off >= edev->size))
> +               return 0;
> +       if ((off + count) > edev->size)
> +               count = edev->size - off;
> +       if (unlikely(!count))
> +               return count;
>
>         cmd_addr = OP_READ << edev->addrlen;
>
> @@ -94,6 +99,7 @@ eeprom_93xx46_bin_read(struct file *filp, struct kobject *kobj,
>         return ret ? : count;
>  }
>
> +

Stray newline?

>  static int eeprom_93xx46_ew(struct eeprom_93xx46_dev *edev, int is_on)
>  {
>         struct spi_message m;
> @@ -182,16 +188,17 @@ eeprom_93xx46_write_word(struct eeprom_93xx46_dev *edev,
>  }
>
>  static ssize_t
> -eeprom_93xx46_bin_write(struct file *filp, struct kobject *kobj,
> -                       struct bin_attribute *bin_attr,
> -                       char *buf, loff_t off, size_t count)
> +eeprom_93xx46_write(struct eeprom_93xx46_dev *edev, const char *buf,
> +                   loff_t off, size_t count)
>  {
> -       struct eeprom_93xx46_dev *edev;
> -       struct device *dev;
>         int i, ret, step = 1;
>
> -       dev = container_of(kobj, struct device, kobj);
> -       edev = dev_get_drvdata(dev);
> +       if (unlikely(off >= edev->size))
> +               return -EFBIG;
> +       if ((off + count) > edev->size)
> +               count = edev->size - off;
> +       if (unlikely(!count))
> +               return count;
>
>         /* only write even number of bytes on 16-bit devices */
>         if (edev->addrlen == 6) {
> @@ -228,6 +235,49 @@ eeprom_93xx46_bin_write(struct file *filp, struct kobject *kobj,
>         return ret ? : count;
>  }
>
> +/*
> + * Provide a regmap interface, which is registered with the NVMEM
> + * framework
> +*/
> +static int eeprom_93xx46_regmap_read(void *context, const void *reg,
> +                                    size_t reg_size, void *val,
> +                                    size_t val_size)
> +{
> +       struct eeprom_93xx46_dev *eeprom_93xx46 = context;
> +       off_t offset = *(u32 *)reg;
> +       int err;
> +
> +       err = eeprom_93xx46_read(eeprom_93xx46, val, offset, val_size);
> +       if (err)
> +               return err;
> +       return 0;

Can be:
return eeprom_93xx46_read(eeprom_93xx46, val, offset, val_size);

Allows you to remove the 'err' variable also.

> +}
> +
> +static int eeprom_93xx46_regmap_write(void *context, const void *data,
> +                                     size_t count)
> +{
> +       struct eeprom_93xx46_dev *eeprom_93xx46 = context;
> +       const char *buf;
> +       u32 offset;
> +       size_t len;
> +       int err;
> +
> +       memcpy(&offset, data, sizeof(offset));
> +       buf = (const char *)data + sizeof(offset);
> +       len = count - sizeof(offset);
> +
> +       err = eeprom_93xx46_write(eeprom_93xx46, buf, offset, len);
> +       if (err)
> +               return err;
> +       return 0;

Same here.


regards,
Joachim Eastwood