Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1335933 > unrolled thread

[PATCHv4 2/7] nvmem: Add backwards compatibility support for older EEPROM drivers.

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCHv4 2/7] nvmem: Add backwards compatibility support for older EEPROM drivers. Andrew Lunn <andrew@lunn.ch> - 2016-02-17 00:50 +0100
    Re: [PATCHv4 2/7] nvmem: Add backwards compatibility support for  older EEPROM drivers. Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2016-02-17 11:30 +0100
      Re: [PATCHv4 2/7] nvmem: Add backwards compatibility support for  older EEPROM drivers. Andrew Lunn <andrew@lunn.ch> - 2016-02-17 14:30 +0100

#1335933 — [PATCHv4 2/7] nvmem: Add backwards compatibility support for older EEPROM drivers.

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@lunn.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;
 };
 
+#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 */
-- 
2.7.0

[toc] | [next] | [standalone]

#1336212 — 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@lunn.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 */
>

[toc] | [prev] | [next] | [standalone]

#1336417 — Re: [PATCHv4 2/7] nvmem: Add backwards compatibility support for older EEPROM drivers.

On Wed, Feb 17, 2016 at 10:17:34AM +0000, Srinivas Kandagatla wrote:
> 
> 
> 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@lunn.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.

Hi Srinivas

For backwards compatibility, we need the 'eeprom' file to be in the
i2c or spi device directory under /sys. This is the base_dev.  The
nvmem file should be in the /sys/class/nvmem directory. This is the
dev in nvmem_device structure. I don't think you can merge them. 

e.g. consider:

        nvmem->dev.type = &nvmem_provider_type;
        nvmem->dev.bus = &nvmem_bus_type;
        nvmem->dev.parent = config->dev;
        np = config->dev->of_node;

The base device is an i2c device on an i2c bus. Would it work if it
suddenly became an nvmem_provider on an nvmem_bus?

	 Thanks

		Andrew

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web