Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965044AbbLOKGM (ORCPT ); Tue, 15 Dec 2015 05:06:12 -0500 Received: from mail-wm0-f45.google.com ([74.125.82.45]:36508 "EHLO mail-wm0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964803AbbLOKFL (ORCPT ); Tue, 15 Dec 2015 05:05:11 -0500 Message-ID: <566FE5D4.1060601@linaro.org> Date: Tue, 15 Dec 2015 10:05:08 +0000 From: Srinivas Kandagatla User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Andrew Lunn , GregKH , maxime.ripard@free-electrons.com, wsa@the-dreams.de, broonie@kernel.org, vz@mleia.com CC: afd@ti.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH 6/6] eeprom: 93xx46: extend driver to plug into the NVMEM framework References: <1449583511-22521-1-git-send-email-andrew@lunn.ch> <1449583511-22521-7-git-send-email-andrew@lunn.ch> In-Reply-To: <1449583511-22521-7-git-send-email-andrew@lunn.ch> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7587 Lines: 242 On 08/12/15 14:05, Andrew Lunn wrote: > Add a regmap for accessing the EEPROM, and then use that with the > NVMEM framework. Use it backward compatibility register function, so > that the 'eeprom' file in sys is provided by the framework. > > Signed-off-by: Andrew Lunn > --- > drivers/misc/eeprom/Kconfig | 3 + > drivers/misc/eeprom/eeprom_93xx46.c | 121 ++++++++++++++++++++++++++++-------- > 2 files changed, 98 insertions(+), 26 deletions(-) > > diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig > index ff3e1dd751ec..5dfa2c2c2277 100644 > --- a/drivers/misc/eeprom/Kconfig > +++ b/drivers/misc/eeprom/Kconfig > @@ -80,6 +80,9 @@ config EEPROM_93CX6 > config EEPROM_93XX46 > tristate "Microwire EEPROM 93XX46 support" > depends on SPI && SYSFS > + select REGMAP > + select NVMEM > + select NVMEM_COMPAT > 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..aa75f59abfba 100644 > --- a/drivers/misc/eeprom/eeprom_93xx46.c > +++ b/drivers/misc/eeprom/eeprom_93xx46.c > @@ -15,7 +15,8 @@ > #include > #include > #include > -#include > +#include > +#include > #include > > #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; > } > > + > 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; > +} > + > +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; > +} > + > +static const struct regmap_bus eeprom_93xx46_regmap_bus = { > + .read = eeprom_93xx46_regmap_read, > + .write = eeprom_93xx46_regmap_write, > + .reg_format_endian_default = REGMAP_ENDIAN_NATIVE, > +}; > + > static int eeprom_93xx46_eral(struct eeprom_93xx46_dev *edev) > { > struct eeprom_93xx46_platform_data *pd = edev->pdata; > @@ -298,6 +348,7 @@ static int eeprom_93xx46_probe(struct spi_device *spi) > { > struct eeprom_93xx46_platform_data *pd; > struct eeprom_93xx46_dev *edev; > + struct regmap *regmap; > int err; > > pd = spi->dev.platform_data; > @@ -325,19 +376,36 @@ static int eeprom_93xx46_probe(struct spi_device *spi) > edev->spi = spi_dev_get(spi); > edev->pdata = pd; > > - sysfs_bin_attr_init(&edev->bin); > - edev->bin.attr.name = "eeprom"; > - edev->bin.attr.mode = S_IRUSR; > - edev->bin.read = eeprom_93xx46_bin_read; > - edev->bin.size = 128; > + edev->size = 128; > + > if (!(pd->flags & EE_READONLY)) { > - edev->bin.write = eeprom_93xx46_bin_write; > - edev->bin.attr.mode |= S_IWUSR; > } > > - err = sysfs_create_bin_file(&spi->dev.kobj, &edev->bin); > - if (err) > + edev->regmap_config.reg_bits = 32; > + edev->regmap_config.val_bits = 8; > + edev->regmap_config.reg_stride = 1; > + edev->regmap_config.max_register = edev->size - 1; > + > + regmap = devm_regmap_init(&spi->dev, &eeprom_93xx46_regmap_bus, edev, > + &edev->regmap_config); > + if (IS_ERR(regmap)) { > + dev_err(&spi->dev, "regmap init failed\n"); > + err = PTR_ERR(regmap); > goto fail; > + } > + > + edev->nvmem_config.name = dev_name(&spi->dev); > + edev->nvmem_config.dev = &spi->dev; > + edev->nvmem_config.read_only = pd->flags & EE_READONLY; > + edev->nvmem_config.root_only = true; > + edev->nvmem_config.owner = THIS_MODULE; > + > + edev->nvmem = nvmem_register_compat(&edev->nvmem_config, > + &spi->dev); Is there a reason for this driver to be using the old style? I can understand the issues with at24/at25 but does this driver also suffer from such issues? IMO, If its possible we should discourage the compat api as much we can. > + if (IS_ERR(edev->nvmem)) { > + err = PTR_ERR(edev->nvmem); > + goto fail; > + } > > dev_info(&spi->dev, "%d-bit eeprom %s\n", > (pd->flags & EE_ADDR8) ? 8 : 16, > @@ -359,10 +427,11 @@ static int eeprom_93xx46_remove(struct spi_device *spi) > { > struct eeprom_93xx46_dev *edev = spi_get_drvdata(spi); > > + nvmem_unregister(edev->nvmem); > + > if (!(edev->pdata->flags & EE_READONLY)) > device_remove_file(&spi->dev, &dev_attr_erase); > > - sysfs_remove_bin_file(&spi->dev.kobj, &edev->bin); > kfree(edev); > return 0; > } > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/