Re: [PATCH] i2c: eeprom: at24: Provide an EEPROM framework interface

Maxime Ripard
2015-05-13T20:40:08Z

[-- Attachment #1: Type: text/plain, Size: 6313 bytes --]

Hi Pantelis,

On Wed, May 13, 2015 at 10:38:17AM +0300, Pantelis Antoniou wrote:
> For DT and in-kernel users there is no interface to the
> at24 EEPROMs so provide an EEPROM framework interface.
>
> This allows us to use AT24 based EEPROMs and reference them
> from within the DT tree.
>
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>

You should probably mention that the EEPROM framework hasn't been
merged yet.

> ---
> drivers/misc/eeprom/at24.c | 222 +++++++++++++++++++++++++++++++++++----------
> 1 file changed, 174 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 2d3db81..afa719a 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -3,6 +3,7 @@
> *
> * Copyright (C) 2005-2007 David Brownell
> * Copyright (C) 2008 Wolfram Sang, Pengutronix
> + * Copyright (C) 2015 Pantelis Antoniou, Konsulko Group
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> @@ -23,6 +24,9 @@
> #include <linux/of.h>
> #include <linux/i2c.h>
> #include <linux/platform_data/at24.h>
> +#include <linux/regmap.h>
> +#include <linux/eeprom-provider.h>
> +#include <linux/io.h>
>
> /*
> * I2C EEPROMs from most vendors are inexpensive and mostly interchangeable.
> @@ -63,12 +67,16 @@ struct at24_data {
> * but not from changes by other I2C masters.
> */
> struct mutex lock;
> - struct bin_attribute bin;
>
> u8 *writebuf;
> unsigned write_max;
> unsigned num_addresses;
>
> + struct regmap_config *regmap_config;
> + struct regmap *regmap;
> + struct eeprom_config *eeprom_config;
> + struct eeprom_device *eeprom_dev;
> +
> /*
> * Some chips tie up multiple I2C addresses; dummy devices reserve
> * them for us, and we'll use them with SMBus calls.
> @@ -131,6 +139,8 @@ static const struct i2c_device_id at24_ids[] = {
> };
> MODULE_DEVICE_TABLE(i2c, at24_ids);
>
> +static DEFINE_IDA(at24_ida);
> +
> /*-------------------------------------------------------------------------*/
>
> /*
> @@ -301,17 +311,6 @@ static ssize_t at24_read(struct at24_data *at24,
> return retval;
> }
>
> -static ssize_t at24_bin_read(struct file *filp, struct kobject *kobj,
> - struct bin_attribute *attr,
> - char *buf, loff_t off, size_t count)
> -{
> - struct at24_data *at24;
> -
> - at24 = dev_get_drvdata(container_of(kobj, struct device, kobj));
> - return at24_read(at24, buf, off, count);
> -}
> -
> -
> /*
> * Note that if the hardware write-protect pin is pulled high, the whole
> * chip is normally write protected. But there are plenty of product
> @@ -432,21 +431,10 @@ static ssize_t at24_write(struct at24_data *at24, const char *buf, loff_t off,
> return retval;
> }
>
> -static ssize_t at24_bin_write(struct file *filp, struct kobject *kobj,
> - struct bin_attribute *attr,
> - char *buf, loff_t off, size_t count)
> -{
> - struct at24_data *at24;
> -
> - if (unlikely(off >= attr->size))
> - return -EFBIG;
> -
> - at24 = dev_get_drvdata(container_of(kobj, struct device, kobj));
> - return at24_write(at24, buf, off, count);
> -}
> -
> /*-------------------------------------------------------------------------*/
>
> +/* panto: using the EEPROM framework macc is superfluous */
> +

Is it? it was kind of one of the point to be able to remove the memory
accessors stuff (which is only used by that driver btw).

> /*
> * This lets other kernel code access the eeprom data. For example, it
> * might hold a board's Ethernet address, or board-specific calibration
> @@ -492,6 +480,91 @@ static void at24_get_ofdata(struct i2c_client *client,
> { }
> #endif /* CONFIG_OF */
>
> +static int regmap_at24_read(void *context,
> + const void *reg, size_t reg_size,
> + void *val, size_t val_size)
> +{
> + struct i2c_client *client = context;
> + struct at24_data *at24;
> + unsigned int offset;
> + int ret;
> +
> + /* reg bits is hardcoded to 32 bits */
> + BUG_ON(reg_size != 4);
> + offset = __raw_readl(reg);
> +
> + at24 = i2c_get_clientdata(client);
> + if (at24 == NULL)
> + return -ENODEV;
> +
> + /* val bytes is always 1 */
> + BUG_ON(at24->regmap_config->val_bits != 8);
> +
> + ret = at24_read(at24, val, offset, val_size);
> + if (ret < 0)
> + return ret;
> + if (ret != val_size)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int regmap_at24_gather_write(void *context,
> + const void *reg, size_t reg_size,
> + const void *val, size_t val_size)
> +{
> + struct i2c_client *client = context;
> + struct at24_data *at24;
> + unsigned int offset;
> + int ret;
> +
> + at24 = i2c_get_clientdata(client);
> + if (at24 == NULL)
> + return -ENODEV;
> +
> + BUG_ON(reg_size != 4);
> + offset = __raw_readl(reg);
> +
> + /* val bytes is always 1 */
> + BUG_ON(at24->regmap_config->val_bits != 8);
> +
> + ret = at24_write(at24, val, offset, val_size);
> + if (ret < 0)
> + return ret;
> + if (ret != val_size)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int regmap_at24_write(void *context, const void *data, size_t count)
> +{
> + struct i2c_client *client = context;
> + struct at24_data *at24;
> + unsigned int reg_bytes, offset;
> +
> + at24 = i2c_get_clientdata(client);
> + if (at24 == NULL)
> + return -ENODEV;
> +
> + reg_bytes = at24->regmap_config->reg_bits / 8;
> + offset = reg_bytes;
> +
> + BUG_ON(reg_bytes != 4);
> + BUG_ON(count <= offset);
> +
> + return regmap_at24_gather_write(context, data, reg_bytes,
> + data + offset, count - offset);
> +}
> +
> +static struct regmap_bus regmap_at24_bus = {
> + .read = regmap_at24_read,
> + .write = regmap_at24_write,
> + .gather_write = regmap_at24_gather_write,
> + .reg_format_endian_default = REGMAP_ENDIAN_NATIVE,
> + .val_format_endian_default = REGMAP_ENDIAN_NATIVE,

I wouldn't expect the content of the EEPROM to change of endianness
when we change the kernel's.

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]