Below are few comments.

On 08/12/15 14:05, 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 <and...@lunn.ch>
---
  drivers/nvmem/Kconfig          |  7 ++++
  drivers/nvmem/core.c           | 75 +++++++++++++++++++++++++++++++++++++++---
  include/linux/nvmem-provider.h | 10 ++++++
  3 files changed, 88 insertions(+), 4 deletions(-)

diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index bc4ea585b42e..b4e79ba7d502 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -13,6 +13,13 @@ menuconfig NVMEM
          If unsure, say no.

  if NVMEM
+config NVMEM_COMPAT
+       bool "Enable /sys compatibility with old eeprom drivers"
+       help
+        Older EEPROM drivers, such as AT24, AT25, provide access to
+        the eeprom via a file called "eeprom" in /sys under the
+        device node. Enabling this option makes the NVMEM core
+        provide this file to retain backwards compatibility

Lets get rid of this Kconfig as Wolfram suggested.
We are already adding NVMEM_COMPAT in the nvmem_device structrure lets move the flags into the struct nvmem_config and use the nvmem_register api as it is.

nvmem_register() can decide what do do with that from there.
I would also prefer a warning if this flag is set, this is to deter any new users.

Let me know your thoughts?

  config NVMEM_IMX_OCOTP
        tristate "i.MX6 On-Chip OTP Controller support"
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 4ccf03da6467..75a498f5e139 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;
@@ -62,10 +67,16 @@ 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 +98,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;
@@ -421,6 +438,53 @@ struct nvmem_device *nvmem_register(const struct 
nvmem_config *config)
  }
  EXPORT_SYMBOL_GPL(nvmem_register);

+#if IS_ENABLED(CONFIG_NVMEM_COMPAT)
+/**
+ * nvmem_register_compat() - Register a nvmem device for given nvmem_config.
+ * Also creates an binary entry in /sys/bus/nvmem/devices/dev-name/nvmem and
+ * an eeprom file in the drivers sys directory.
+ *
+ * @config: nvmem device configuration with which nvmem device is created.
+ * @dev: device structure of underlying device
+ *
+ * Return: Will be an ERR_PTR() on error or a valid pointer to nvmem_device
+ * on success.
+ */
+
+struct nvmem_device *nvmem_register_compat(const struct nvmem_config *config,
+                                          struct device *base_dev)
+{
Possibly move most of it or some of it a local static function which will be called from nvmem_register depending on the NVMEM_FLAG_COMPAT.

+       struct nvmem_device *nvmem;
+       int rval;
+
+       nvmem = nvmem_register(config);
+       if (IS_ERR(nvmem))
+               return nvmem;
+
+       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;
+       nvmem->eeprom.private = &nvmem->dev;
+       nvmem->base_dev = 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);
+               nvmem_unregister(nvmem);
+               return ERR_PTR(rval);
+       }
+
+       nvmem->flags |= FLAG_COMPAT;
+
+       return nvmem;
+}
+EXPORT_SYMBOL_GPL(nvmem_register_compat);
+#endif /* CONFIG_NVMEM_COMPAT */
+
  /**
   * nvmem_unregister() - Unregister previously registered nvmem device
   *
@@ -437,6 +501,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..012030bd4495 100644
--- a/include/linux/nvmem-provider.h
+++ b/include/linux/nvmem-provider.h
@@ -45,4 +45,14 @@ static inline int nvmem_unregister(struct nvmem_device 
*nvmem)

  #endif /* CONFIG_NVMEM */

+#if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_NVMEM_COMPAT)
+struct nvmem_device *nvmem_register_compat(const struct nvmem_config *config,
+                                          struct device *base_dev);
+#else
+static inline struct nvmem_device *
+nvmem_register_compat(const struct nvmem_config *c, struct device *base_dev)
+{
+       return ERR_PTR(-ENOSYS);
+}
+#endif  /* CONFIG_NVMEM && CONFIG_NVMEM_COMPAT */
  #endif  /* ifndef _LINUX_NVMEM_PROVIDER_H */

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to