[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <56C448BE.30303@linaro.org>
Date: Wed, 17 Feb 2016 10:17:34 +0000
From: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To: Andrew Lunn <andrew@...n.ch>, GregKH <greg@...ah.com>
Cc: maxime.ripard@...e-electrons.com, wsa@...-dreams.de,
broonie@...nel.org, vz@...ia.com, fd@...com,
linux-kernel@...r.kernel.org, pantelis.antoniou@...sulko.com,
bgolaszewski@...libre.com
Subject: Re: [PATCHv4 2/7] nvmem: Add backwards compatibility support for
older EEPROM drivers.
On 16/02/16 23:41, Andrew Lunn wrote:
> Older drivers made an 'eeprom' file available in the /sys device
> directory. Have the NVMEM core provide this to retain backwards
> compatibility.
>
> Signed-off-by: Andrew Lunn <andrew@...n.ch>
> ---
> v4: Add lockdep support
> ---
> drivers/nvmem/core.c | 84 ++++++++++++++++++++++++++++++++++++++----
> include/linux/nvmem-provider.h | 4 +-
> 2 files changed, 79 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 4ccf03da6467..9ad1c2cf75ac 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -38,8 +38,13 @@ struct nvmem_device {
> int users;
> size_t size;
> bool read_only;
> + int flags;
> + struct bin_attribute eeprom;
> + struct device *base_dev;
Any reason why should this base_dev be any different to the dev in the
nvmem_config?
Should we just not reuse dev? unless am missing something obvious.
Other parts of the patch looks good to me.
> };
>
> +#define FLAG_COMPAT BIT(0)
> +
> struct nvmem_cell {
> const char *name;
> int offset;
> @@ -56,16 +61,26 @@ static DEFINE_IDA(nvmem_ida);
> static LIST_HEAD(nvmem_cells);
> static DEFINE_MUTEX(nvmem_cells_mutex);
>
> +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> +static struct lock_class_key eeprom_lock_key;
> +#endif
> +
> #define to_nvmem_device(d) container_of(d, struct nvmem_device, dev)
>
> static ssize_t bin_attr_nvmem_read(struct file *filp, struct kobject *kobj,
> struct bin_attribute *attr,
> char *buf, loff_t pos, size_t count)
> {
> - struct device *dev = container_of(kobj, struct device, kobj);
> - struct nvmem_device *nvmem = to_nvmem_device(dev);
> + struct device *dev;
> + struct nvmem_device *nvmem;
> int rc;
>
> + if (attr->private)
> + dev = attr->private;
> + else
> + dev = container_of(kobj, struct device, kobj);
> + nvmem = to_nvmem_device(dev);
> +
> /* Stop the user from reading */
> if (pos >= nvmem->size)
> return 0;
> @@ -87,10 +102,16 @@ static ssize_t bin_attr_nvmem_write(struct file *filp, struct kobject *kobj,
> struct bin_attribute *attr,
> char *buf, loff_t pos, size_t count)
> {
> - struct device *dev = container_of(kobj, struct device, kobj);
> - struct nvmem_device *nvmem = to_nvmem_device(dev);
> + struct device *dev;
> + struct nvmem_device *nvmem;
> int rc;
>
> + if (attr->private)
> + dev = attr->private;
> + else
> + dev = container_of(kobj, struct device, kobj);
> + nvmem = to_nvmem_device(dev);
> +
> /* Stop the user from writing */
> if (pos >= nvmem->size)
> return 0;
> @@ -341,6 +362,43 @@ err:
> return rval;
> }
>
> +/*
> + * nvmem_setup_compat() - Create an additional binary entry in
> + * drivers sys directory, to be backwards compatible with the older
> + * drivers/misc/eeprom drivers.
> + */
> +static int nvmem_setup_compat(struct nvmem_device *nvmem,
> + const struct nvmem_config *config)
> +{
> + int rval;
> +
> + if (!config->base_dev)
> + return -EINVAL;
> +
> + if (nvmem->read_only)
> + nvmem->eeprom = bin_attr_ro_root_nvmem;
> + else
> + nvmem->eeprom = bin_attr_rw_root_nvmem;
> + nvmem->eeprom.attr.name = "eeprom";
> + nvmem->eeprom.size = nvmem->size;
> +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> + nvmem->eeprom.attr.key = &eeprom_lock_key;
> +#endif
> + nvmem->eeprom.private = &nvmem->dev;
> + nvmem->base_dev = config->base_dev;
> +
> + rval = device_create_bin_file(nvmem->base_dev, &nvmem->eeprom);
> + if (rval) {
> + dev_err(&nvmem->dev,
> + "Failed to create eeprom binary file %d\n", rval);
> + return rval;
> + }
> +
> + nvmem->flags |= FLAG_COMPAT;
> +
> + return 0;
> +}
> +
> /**
> * nvmem_register() - Register a nvmem device for given nvmem_config.
> * Also creates an binary entry in /sys/bus/nvmem/devices/dev-name/nvmem
> @@ -408,16 +466,23 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
> dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name);
>
> rval = device_add(&nvmem->dev);
> - if (rval) {
> - ida_simple_remove(&nvmem_ida, nvmem->id);
> - kfree(nvmem);
> - return ERR_PTR(rval);
> + if (rval)
> + goto out;
> +
> + if (config->compat) {
> + rval = nvmem_setup_compat(nvmem, config);
> + if (rval)
> + goto out;
> }
>
> if (config->cells)
> nvmem_add_cells(nvmem, config);
>
> return nvmem;
> +out:
> + ida_simple_remove(&nvmem_ida, nvmem->id);
> + kfree(nvmem);
> + return ERR_PTR(rval);
> }
> EXPORT_SYMBOL_GPL(nvmem_register);
>
> @@ -437,6 +502,9 @@ int nvmem_unregister(struct nvmem_device *nvmem)
> }
> mutex_unlock(&nvmem_mutex);
>
> + if (nvmem->flags & FLAG_COMPAT)
> + device_remove_bin_file(nvmem->base_dev, &nvmem->eeprom);
> +
> nvmem_device_remove_all_cells(nvmem);
> device_del(&nvmem->dev);
>
> diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
> index d24fefa0c11d..a4fcc90b0f20 100644
> --- a/include/linux/nvmem-provider.h
> +++ b/include/linux/nvmem-provider.h
> @@ -24,6 +24,9 @@ struct nvmem_config {
> int ncells;
> bool read_only;
> bool root_only;
> + /* To be only used by old driver/misc/eeprom drivers */
> + bool compat;
> + struct device *base_dev;
> };
>
> #if IS_ENABLED(CONFIG_NVMEM)
> @@ -44,5 +47,4 @@ static inline int nvmem_unregister(struct nvmem_device *nvmem)
> }
>
> #endif /* CONFIG_NVMEM */
> -
> #endif /* ifndef _LINUX_NVMEM_PROVIDER_H */
>
Powered by blists - more mailing lists