| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 15 Dec 2015 10:05:08 +0000
From: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To: Andrew Lunn <andrew@...n.ch>, GregKH <greg@...ah.com>,
maxime.ripard@...e-electrons.com, wsa@...-dreams.de,
broonie@...nel.org, vz@...ia.com
CC: afd@...com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 6/6] eeprom: 93xx46: extend driver to plug into the NVMEM
framework
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 <andrew@...n.ch>
> ---
> 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 <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;
> }
>
> +
> 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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists