[PATCH] nvmem: core: add NVMEM_SYSFS Kconfig

[PATCH] nvmem: core: add NVMEM_SYSFS Kconfig

[Srinivas Kandagatla]

Many nvmem providers are not very keen on having default sysfs
nvmem entry, as most of the usecases for them are inside kernel
itself. And in some cases read/writes to some areas in nvmem are
restricted and trapped at secure monitor level, so accessing them
from userspace would result in board reboots.

This patch adds new NVMEM_SYSFS Kconfig to make binary sysfs entry
an optional one. This provision will give more flexibility to users.
This patch also moves existing sysfs code to a new file so that its
not compiled in when its not really required.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 Documentation/ABI/stable/sysfs-bus-nvmem |   2 +
 drivers/nvmem/Kconfig                    |   9 +
 drivers/nvmem/Makefile                   |   3 +
 drivers/nvmem/core.c                     | 264 +----------------------
 drivers/nvmem/nvmem-sysfs.c              | 256 ++++++++++++++++++++++
 drivers/nvmem/nvmem.h                    |  62 ++++++
 6 files changed, 336 insertions(+), 260 deletions(-)
 create mode 100644 drivers/nvmem/nvmem-sysfs.c
 create mode 100644 drivers/nvmem/nvmem.h

diff --git a/Documentation/ABI/stable/sysfs-bus-nvmem b/Documentation/ABI/stable/sysfs-bus-nvmem
index 5923ab4620c5..9ffba8576f7b 100644
--- a/Documentation/ABI/stable/sysfs-bus-nvmem
+++ b/Documentation/ABI/stable/sysfs-bus-nvmem
@@ -6,6 +6,8 @@ Description:
 		This file allows user to read/write the raw NVMEM contents.
 		Permissions for write to this file depends on the nvmem
 		provider configuration.
+		Note: This file is only present if CONFIG_NVMEM_SYSFS
+		is enabled
 
 		ex:
 		hexdump /sys/bus/nvmem/devices/qfprom0/nvmem
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index a90e9a1ebe55..e316811e9c04 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -13,6 +13,15 @@ menuconfig NVMEM
 
 if NVMEM
 
+config NVMEM_SYSFS
+	bool "/sys/bus/nvmem/devices/*/nvmem (sysfs interface)"
+	depends on SYSFS
+	help
+	 Say Y here to add a sysfs interface for NVMEM.
+
+	 This interface is mostly used by userspace applications to
+	 read/write directly into nvmem.
+
 config NVMEM_IMX_IIM
 	tristate "i.MX IC Identification Module support"
 	depends on ARCH_MXC || COMPILE_TEST
diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
index 4c7ba12a7005..c1fe4768dfef 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -6,6 +6,9 @@
 obj-$(CONFIG_NVMEM)		+= nvmem_core.o
 nvmem_core-y			:= core.o
 
+obj-$(CONFIG_NVMEM_SYSFS)	+= nvmem_sysfs.o
+nvmem_sysfs-y			:= nvmem-sysfs.o
+
 # Devices
 obj-$(CONFIG_NVMEM_BCM_OCOTP)	+= nvmem-bcm-ocotp.o
 nvmem-bcm-ocotp-y		:= bcm-ocotp.o
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 5abebf2128b8..c7892c3da91f 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -17,27 +17,7 @@
 #include <linux/nvmem-provider.h>
 #include <linux/of.h>
 #include <linux/slab.h>
-
-struct nvmem_device {
-	struct module		*owner;
-	struct device		dev;
-	int			stride;
-	int			word_size;
-	int			id;
-	struct kref		refcnt;
-	size_t			size;
-	bool			read_only;
-	int			flags;
-	enum nvmem_type		type;
-	struct bin_attribute	eeprom;
-	struct device		*base_dev;
-	struct list_head	cells;
-	nvmem_reg_read_t	reg_read;
-	nvmem_reg_write_t	reg_write;
-	void *priv;
-};
-
-#define FLAG_COMPAT		BIT(0)
+#include "nvmem.h"
 
 struct nvmem_cell {
 	const char		*name;
@@ -61,18 +41,7 @@ static LIST_HEAD(nvmem_lookup_list);
 
 static BLOCKING_NOTIFIER_HEAD(nvmem_notifier);
 
-static const char * const nvmem_type_str[] = {
-	[NVMEM_TYPE_UNKNOWN] = "Unknown",
-	[NVMEM_TYPE_EEPROM] = "EEPROM",
-	[NVMEM_TYPE_OTP] = "OTP",
-	[NVMEM_TYPE_BATTERY_BACKED] = "Battery backed",
-};
-
-#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 int nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
 			  void *val, size_t bytes)
 {
@@ -91,187 +60,6 @@ static int nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset,
 	return -EINVAL;
 }
 
-static ssize_t type_show(struct device *dev,
-			 struct device_attribute *attr, char *buf)
-{
-	struct nvmem_device *nvmem = to_nvmem_device(dev);
-
-	return sprintf(buf, "%s\n", nvmem_type_str[nvmem->type]);
-}
-
-static DEVICE_ATTR_RO(type);
-
-static struct attribute *nvmem_attrs[] = {
-	&dev_attr_type.attr,
-	NULL,
-};
-
-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;
-	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;
-
-	if (count < nvmem->word_size)
-		return -EINVAL;
-
-	if (pos + count > nvmem->size)
-		count = nvmem->size - pos;
-
-	count = round_down(count, nvmem->word_size);
-
-	rc = nvmem_reg_read(nvmem, pos, buf, count);
-
-	if (rc)
-		return rc;
-
-	return count;
-}
-
-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;
-	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 -EFBIG;
-
-	if (count < nvmem->word_size)
-		return -EINVAL;
-
-	if (pos + count > nvmem->size)
-		count = nvmem->size - pos;
-
-	count = round_down(count, nvmem->word_size);
-
-	rc = nvmem_reg_write(nvmem, pos, buf, count);
-
-	if (rc)
-		return rc;
-
-	return count;
-}
-
-/* default read/write permissions */
-static struct bin_attribute bin_attr_rw_nvmem = {
-	.attr	= {
-		.name	= "nvmem",
-		.mode	= 0644,
-	},
-	.read	= bin_attr_nvmem_read,
-	.write	= bin_attr_nvmem_write,
-};
-
-static struct bin_attribute *nvmem_bin_rw_attributes[] = {
-	&bin_attr_rw_nvmem,
-	NULL,
-};
-
-static const struct attribute_group nvmem_bin_rw_group = {
-	.bin_attrs	= nvmem_bin_rw_attributes,
-	.attrs		= nvmem_attrs,
-};
-
-static const struct attribute_group *nvmem_rw_dev_groups[] = {
-	&nvmem_bin_rw_group,
-	NULL,
-};
-
-/* read only permission */
-static struct bin_attribute bin_attr_ro_nvmem = {
-	.attr	= {
-		.name	= "nvmem",
-		.mode	= 0444,
-	},
-	.read	= bin_attr_nvmem_read,
-};
-
-static struct bin_attribute *nvmem_bin_ro_attributes[] = {
-	&bin_attr_ro_nvmem,
-	NULL,
-};
-
-static const struct attribute_group nvmem_bin_ro_group = {
-	.bin_attrs	= nvmem_bin_ro_attributes,
-	.attrs		= nvmem_attrs,
-};
-
-static const struct attribute_group *nvmem_ro_dev_groups[] = {
-	&nvmem_bin_ro_group,
-	NULL,
-};
-
-/* default read/write permissions, root only */
-static struct bin_attribute bin_attr_rw_root_nvmem = {
-	.attr	= {
-		.name	= "nvmem",
-		.mode	= 0600,
-	},
-	.read	= bin_attr_nvmem_read,
-	.write	= bin_attr_nvmem_write,
-};
-
-static struct bin_attribute *nvmem_bin_rw_root_attributes[] = {
-	&bin_attr_rw_root_nvmem,
-	NULL,
-};
-
-static const struct attribute_group nvmem_bin_rw_root_group = {
-	.bin_attrs	= nvmem_bin_rw_root_attributes,
-	.attrs		= nvmem_attrs,
-};
-
-static const struct attribute_group *nvmem_rw_root_dev_groups[] = {
-	&nvmem_bin_rw_root_group,
-	NULL,
-};
-
-/* read only permission, root only */
-static struct bin_attribute bin_attr_ro_root_nvmem = {
-	.attr	= {
-		.name	= "nvmem",
-		.mode	= 0400,
-	},
-	.read	= bin_attr_nvmem_read,
-};
-
-static struct bin_attribute *nvmem_bin_ro_root_attributes[] = {
-	&bin_attr_ro_root_nvmem,
-	NULL,
-};
-
-static const struct attribute_group nvmem_bin_ro_root_group = {
-	.bin_attrs	= nvmem_bin_ro_root_attributes,
-	.attrs		= nvmem_attrs,
-};
-
-static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
-	&nvmem_bin_ro_root_group,
-	NULL,
-};
-
 static void nvmem_release(struct device *dev)
 {
 	struct nvmem_device *nvmem = to_nvmem_device(dev);
@@ -422,43 +210,6 @@ static int nvmem_add_cells(struct nvmem_device *nvmem,
 	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_notifier() - Register a notifier block for nvmem events.
  *
@@ -651,14 +402,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 	nvmem->read_only = device_property_present(config->dev, "read-only") ||
 			   config->read_only || !nvmem->reg_write;
 
-	if (config->root_only)
-		nvmem->dev.groups = nvmem->read_only ?
-			nvmem_ro_root_dev_groups :
-			nvmem_rw_root_dev_groups;
-	else
-		nvmem->dev.groups = nvmem->read_only ?
-			nvmem_ro_dev_groups :
-			nvmem_rw_dev_groups;
+	nvmem->dev.groups = nvmem_sysfs_get_groups(nvmem, config);
 
 	device_initialize(&nvmem->dev);
 
@@ -669,7 +413,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 		goto err_put_device;
 
 	if (config->compat) {
-		rval = nvmem_setup_compat(nvmem, config);
+		rval = nvmem_sysfs_setup_compat(nvmem, config);
 		if (rval)
 			goto err_device_del;
 	}
@@ -696,7 +440,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 	nvmem_device_remove_all_cells(nvmem);
 err_teardown_compat:
 	if (config->compat)
-		device_remove_bin_file(nvmem->base_dev, &nvmem->eeprom);
+		nvmem_sysfs_remove_compat(nvmem, config);
 err_device_del:
 	device_del(&nvmem->dev);
 err_put_device:
diff --git a/drivers/nvmem/nvmem-sysfs.c b/drivers/nvmem/nvmem-sysfs.c
new file mode 100644
index 000000000000..6f303b91f6e7
--- /dev/null
+++ b/drivers/nvmem/nvmem-sysfs.c
@@ -0,0 +1,256 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019, Linaro Limited
+ */
+#include "nvmem.h"
+
+static const char * const nvmem_type_str[] = {
+	[NVMEM_TYPE_UNKNOWN] = "Unknown",
+	[NVMEM_TYPE_EEPROM] = "EEPROM",
+	[NVMEM_TYPE_OTP] = "OTP",
+	[NVMEM_TYPE_BATTERY_BACKED] = "Battery backed",
+};
+
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+static struct lock_class_key eeprom_lock_key;
+#endif
+
+static ssize_t type_show(struct device *dev,
+			 struct device_attribute *attr, char *buf)
+{
+	struct nvmem_device *nvmem = to_nvmem_device(dev);
+
+	return sprintf(buf, "%s\n", nvmem_type_str[nvmem->type]);
+}
+
+static DEVICE_ATTR_RO(type);
+
+static struct attribute *nvmem_attrs[] = {
+	&dev_attr_type.attr,
+	NULL,
+};
+
+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;
+	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;
+
+	if (count < nvmem->word_size)
+		return -EINVAL;
+
+	if (pos + count > nvmem->size)
+		count = nvmem->size - pos;
+
+	count = round_down(count, nvmem->word_size);
+
+	rc = nvmem->reg_read(nvmem->priv, pos, buf, count);
+
+	if (rc)
+		return rc;
+
+	return count;
+}
+
+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;
+	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 -EFBIG;
+
+	if (count < nvmem->word_size)
+		return -EINVAL;
+
+	if (pos + count > nvmem->size)
+		count = nvmem->size - pos;
+
+	count = round_down(count, nvmem->word_size);
+
+	rc = nvmem->reg_write(nvmem->priv, pos, buf, count);
+
+	if (rc)
+		return rc;
+
+	return count;
+}
+
+/* default read/write permissions */
+static struct bin_attribute bin_attr_rw_nvmem = {
+	.attr	= {
+		.name	= "nvmem",
+		.mode	= 0644,
+	},
+	.read	= bin_attr_nvmem_read,
+	.write	= bin_attr_nvmem_write,
+};
+
+static struct bin_attribute *nvmem_bin_rw_attributes[] = {
+	&bin_attr_rw_nvmem,
+	NULL,
+};
+
+static const struct attribute_group nvmem_bin_rw_group = {
+	.bin_attrs	= nvmem_bin_rw_attributes,
+	.attrs		= nvmem_attrs,
+};
+
+static const struct attribute_group *nvmem_rw_dev_groups[] = {
+	&nvmem_bin_rw_group,
+	NULL,
+};
+
+/* read only permission */
+static struct bin_attribute bin_attr_ro_nvmem = {
+	.attr	= {
+		.name	= "nvmem",
+		.mode	= 0444,
+	},
+	.read	= bin_attr_nvmem_read,
+};
+
+static struct bin_attribute *nvmem_bin_ro_attributes[] = {
+	&bin_attr_ro_nvmem,
+	NULL,
+};
+
+static const struct attribute_group nvmem_bin_ro_group = {
+	.bin_attrs	= nvmem_bin_ro_attributes,
+	.attrs		= nvmem_attrs,
+};
+
+static const struct attribute_group *nvmem_ro_dev_groups[] = {
+	&nvmem_bin_ro_group,
+	NULL,
+};
+
+/* default read/write permissions, root only */
+static struct bin_attribute bin_attr_rw_root_nvmem = {
+	.attr	= {
+		.name	= "nvmem",
+		.mode	= 0600,
+	},
+	.read	= bin_attr_nvmem_read,
+	.write	= bin_attr_nvmem_write,
+};
+
+static struct bin_attribute *nvmem_bin_rw_root_attributes[] = {
+	&bin_attr_rw_root_nvmem,
+	NULL,
+};
+
+static const struct attribute_group nvmem_bin_rw_root_group = {
+	.bin_attrs	= nvmem_bin_rw_root_attributes,
+	.attrs		= nvmem_attrs,
+};
+
+static const struct attribute_group *nvmem_rw_root_dev_groups[] = {
+	&nvmem_bin_rw_root_group,
+	NULL,
+};
+
+/* read only permission, root only */
+static struct bin_attribute bin_attr_ro_root_nvmem = {
+	.attr	= {
+		.name	= "nvmem",
+		.mode	= 0400,
+	},
+	.read	= bin_attr_nvmem_read,
+};
+
+static struct bin_attribute *nvmem_bin_ro_root_attributes[] = {
+	&bin_attr_ro_root_nvmem,
+	NULL,
+};
+
+static const struct attribute_group nvmem_bin_ro_root_group = {
+	.bin_attrs	= nvmem_bin_ro_root_attributes,
+	.attrs		= nvmem_attrs,
+};
+
+static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
+	&nvmem_bin_ro_root_group,
+	NULL,
+};
+
+const struct attribute_group **nvmem_sysfs_get_groups(
+					struct nvmem_device *nvmem,
+					const struct nvmem_config *config)
+{
+	if (config->root_only)
+		return nvmem->read_only ?
+			nvmem_ro_root_dev_groups :
+			nvmem_rw_root_dev_groups;
+
+	return nvmem->read_only ? nvmem_ro_dev_groups : nvmem_rw_dev_groups;
+}
+
+/*
+ * nvmem_setup_compat() - Create an additional binary entry in
+ * drivers sys directory, to be backwards compatible with the older
+ * drivers/misc/eeprom drivers.
+ */
+int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
+			      const struct nvmem_config *config)
+{
+	int rval;
+
+	if (!config->compat)
+		return 0;
+
+	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;
+}
+
+void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem,
+			      const struct nvmem_config *config)
+{
+	if (config->compat)
+		device_remove_bin_file(nvmem->base_dev, &nvmem->eeprom);
+}
diff --git a/drivers/nvmem/nvmem.h b/drivers/nvmem/nvmem.h
new file mode 100644
index 000000000000..eb8ed7121fa3
--- /dev/null
+++ b/drivers/nvmem/nvmem.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _DRIVERS_NVMEM_H
+#define _DRIVERS_NVMEM_H
+
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/kref.h>
+#include <linux/list.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/nvmem-provider.h>
+
+struct nvmem_device {
+	struct module		*owner;
+	struct device		dev;
+	int			stride;
+	int			word_size;
+	int			id;
+	struct kref		refcnt;
+	size_t			size;
+	bool			read_only;
+	int			flags;
+	enum nvmem_type		type;
+	struct bin_attribute	eeprom;
+	struct device		*base_dev;
+	struct list_head	cells;
+	nvmem_reg_read_t	reg_read;
+	nvmem_reg_write_t	reg_write;
+	void *priv;
+};
+
+#define to_nvmem_device(d) container_of(d, struct nvmem_device, dev)
+#define FLAG_COMPAT		BIT(0)
+
+#ifdef CONFIG_NVMEM_SYSFS
+const struct attribute_group **nvmem_sysfs_get_groups(
+					struct nvmem_device *nvmem,
+					const struct nvmem_config *config);
+int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
+			      const struct nvmem_config *config);
+void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem,
+			      const struct nvmem_config *config);
+#else
+static inline const struct attribute_group **nvmem_sysfs_get_groups(
+					struct nvmem_device *nvmem,
+					const struct nvmem_config *config)
+{
+	return NULL;
+}
+
+static inline int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
+				      const struct nvmem_config *config)
+{
+	return -ENOSYS;
+}
+static inline void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem,
+			      const struct nvmem_config *config)
+{
+}
+#endif /* CONFIG_NVMEM_SYSFS */
+
+#endif /* _DRIVERS_NVMEM_H */
-- 
2.21.0

Re: [PATCH] nvmem: core: add NVMEM_SYSFS Kconfig

[Gaurav Kohli]

Hi Srinivas,

Thanks for the patch,
By default NVMEM_SYSFS should be set true, those whose don't want they 
can disable the same.

If we go with disable option, there are chances of eeprom may break in 
below case:

if (config->compat) {
                 rval = nvmem_sysfs_setup_compat(nvmem, config); -> this 
will return error as config is disabled.
                 if (rval)
                         goto err_device_del;
         }

Regards
Gaurav


On 4/15/2019 10:10 PM, Srinivas Kandagatla wrote:
> Many nvmem providers are not very keen on having default sysfs
> nvmem entry, as most of the usecases for them are inside kernel
> itself. And in some cases read/writes to some areas in nvmem are
> restricted and trapped at secure monitor level, so accessing them
> from userspace would result in board reboots.
> 
> This patch adds new NVMEM_SYSFS Kconfig to make binary sysfs entry
> an optional one. This provision will give more flexibility to users.
> This patch also moves existing sysfs code to a new file so that its
> not compiled in when its not really required.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>   Documentation/ABI/stable/sysfs-bus-nvmem |   2 +
>   drivers/nvmem/Kconfig                    |   9 +
>   drivers/nvmem/Makefile                   |   3 +
>   drivers/nvmem/core.c                     | 264 +----------------------
>   drivers/nvmem/nvmem-sysfs.c              | 256 ++++++++++++++++++++++
>   drivers/nvmem/nvmem.h                    |  62 ++++++
>   6 files changed, 336 insertions(+), 260 deletions(-)
>   create mode 100644 drivers/nvmem/nvmem-sysfs.c
>   create mode 100644 drivers/nvmem/nvmem.h
> 
> diff --git a/Documentation/ABI/stable/sysfs-bus-nvmem b/Documentation/ABI/stable/sysfs-bus-nvmem
> index 5923ab4620c5..9ffba8576f7b 100644
> --- a/Documentation/ABI/stable/sysfs-bus-nvmem
> +++ b/Documentation/ABI/stable/sysfs-bus-nvmem
> @@ -6,6 +6,8 @@ Description:
>   		This file allows user to read/write the raw NVMEM contents.
>   		Permissions for write to this file depends on the nvmem
>   		provider configuration.
> +		Note: This file is only present if CONFIG_NVMEM_SYSFS
> +		is enabled
>   
>   		ex:
>   		hexdump /sys/bus/nvmem/devices/qfprom0/nvmem
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index a90e9a1ebe55..e316811e9c04 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -13,6 +13,15 @@ menuconfig NVMEM
>   
>   if NVMEM
>   
> +config NVMEM_SYSFS
> +	bool "/sys/bus/nvmem/devices/*/nvmem (sysfs interface)"
> +	depends on SYSFS
> +	help
> +	 Say Y here to add a sysfs interface for NVMEM.
> +
> +	 This interface is mostly used by userspace applications to
> +	 read/write directly into nvmem.
> +
>   config NVMEM_IMX_IIM
>   	tristate "i.MX IC Identification Module support"
>   	depends on ARCH_MXC || COMPILE_TEST
> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
> index 4c7ba12a7005..c1fe4768dfef 100644
> --- a/drivers/nvmem/Makefile
> +++ b/drivers/nvmem/Makefile
> @@ -6,6 +6,9 @@
>   obj-$(CONFIG_NVMEM)		+= nvmem_core.o
>   nvmem_core-y			:= core.o
>   
> +obj-$(CONFIG_NVMEM_SYSFS)	+= nvmem_sysfs.o
> +nvmem_sysfs-y			:= nvmem-sysfs.o
> +
>   # Devices
>   obj-$(CONFIG_NVMEM_BCM_OCOTP)	+= nvmem-bcm-ocotp.o
>   nvmem-bcm-ocotp-y		:= bcm-ocotp.o
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 5abebf2128b8..c7892c3da91f 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -17,27 +17,7 @@
>   #include <linux/nvmem-provider.h>
>   #include <linux/of.h>
>   #include <linux/slab.h>
> -
> -struct nvmem_device {
> -	struct module		*owner;
> -	struct device		dev;
> -	int			stride;
> -	int			word_size;
> -	int			id;
> -	struct kref		refcnt;
> -	size_t			size;
> -	bool			read_only;
> -	int			flags;
> -	enum nvmem_type		type;
> -	struct bin_attribute	eeprom;
> -	struct device		*base_dev;
> -	struct list_head	cells;
> -	nvmem_reg_read_t	reg_read;
> -	nvmem_reg_write_t	reg_write;
> -	void *priv;
> -};
> -
> -#define FLAG_COMPAT		BIT(0)
> +#include "nvmem.h"
>   
>   struct nvmem_cell {
>   	const char		*name;
> @@ -61,18 +41,7 @@ static LIST_HEAD(nvmem_lookup_list);
>   
>   static BLOCKING_NOTIFIER_HEAD(nvmem_notifier);
>   
> -static const char * const nvmem_type_str[] = {
> -	[NVMEM_TYPE_UNKNOWN] = "Unknown",
> -	[NVMEM_TYPE_EEPROM] = "EEPROM",
> -	[NVMEM_TYPE_OTP] = "OTP",
> -	[NVMEM_TYPE_BATTERY_BACKED] = "Battery backed",
> -};
> -
> -#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 int nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
>   			  void *val, size_t bytes)
>   {
> @@ -91,187 +60,6 @@ static int nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset,
>   	return -EINVAL;
>   }
>   
> -static ssize_t type_show(struct device *dev,
> -			 struct device_attribute *attr, char *buf)
> -{
> -	struct nvmem_device *nvmem = to_nvmem_device(dev);
> -
> -	return sprintf(buf, "%s\n", nvmem_type_str[nvmem->type]);
> -}
> -
> -static DEVICE_ATTR_RO(type);
> -
> -static struct attribute *nvmem_attrs[] = {
> -	&dev_attr_type.attr,
> -	NULL,
> -};
> -
> -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;
> -	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;
> -
> -	if (count < nvmem->word_size)
> -		return -EINVAL;
> -
> -	if (pos + count > nvmem->size)
> -		count = nvmem->size - pos;
> -
> -	count = round_down(count, nvmem->word_size);
> -
> -	rc = nvmem_reg_read(nvmem, pos, buf, count);
> -
> -	if (rc)
> -		return rc;
> -
> -	return count;
> -}
> -
> -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;
> -	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 -EFBIG;
> -
> -	if (count < nvmem->word_size)
> -		return -EINVAL;
> -
> -	if (pos + count > nvmem->size)
> -		count = nvmem->size - pos;
> -
> -	count = round_down(count, nvmem->word_size);
> -
> -	rc = nvmem_reg_write(nvmem, pos, buf, count);
> -
> -	if (rc)
> -		return rc;
> -
> -	return count;
> -}
> -
> -/* default read/write permissions */
> -static struct bin_attribute bin_attr_rw_nvmem = {
> -	.attr	= {
> -		.name	= "nvmem",
> -		.mode	= 0644,
> -	},
> -	.read	= bin_attr_nvmem_read,
> -	.write	= bin_attr_nvmem_write,
> -};
> -
> -static struct bin_attribute *nvmem_bin_rw_attributes[] = {
> -	&bin_attr_rw_nvmem,
> -	NULL,
> -};
> -
> -static const struct attribute_group nvmem_bin_rw_group = {
> -	.bin_attrs	= nvmem_bin_rw_attributes,
> -	.attrs		= nvmem_attrs,
> -};
> -
> -static const struct attribute_group *nvmem_rw_dev_groups[] = {
> -	&nvmem_bin_rw_group,
> -	NULL,
> -};
> -
> -/* read only permission */
> -static struct bin_attribute bin_attr_ro_nvmem = {
> -	.attr	= {
> -		.name	= "nvmem",
> -		.mode	= 0444,
> -	},
> -	.read	= bin_attr_nvmem_read,
> -};
> -
> -static struct bin_attribute *nvmem_bin_ro_attributes[] = {
> -	&bin_attr_ro_nvmem,
> -	NULL,
> -};
> -
> -static const struct attribute_group nvmem_bin_ro_group = {
> -	.bin_attrs	= nvmem_bin_ro_attributes,
> -	.attrs		= nvmem_attrs,
> -};
> -
> -static const struct attribute_group *nvmem_ro_dev_groups[] = {
> -	&nvmem_bin_ro_group,
> -	NULL,
> -};
> -
> -/* default read/write permissions, root only */
> -static struct bin_attribute bin_attr_rw_root_nvmem = {
> -	.attr	= {
> -		.name	= "nvmem",
> -		.mode	= 0600,
> -	},
> -	.read	= bin_attr_nvmem_read,
> -	.write	= bin_attr_nvmem_write,
> -};
> -
> -static struct bin_attribute *nvmem_bin_rw_root_attributes[] = {
> -	&bin_attr_rw_root_nvmem,
> -	NULL,
> -};
> -
> -static const struct attribute_group nvmem_bin_rw_root_group = {
> -	.bin_attrs	= nvmem_bin_rw_root_attributes,
> -	.attrs		= nvmem_attrs,
> -};
> -
> -static const struct attribute_group *nvmem_rw_root_dev_groups[] = {
> -	&nvmem_bin_rw_root_group,
> -	NULL,
> -};
> -
> -/* read only permission, root only */
> -static struct bin_attribute bin_attr_ro_root_nvmem = {
> -	.attr	= {
> -		.name	= "nvmem",
> -		.mode	= 0400,
> -	},
> -	.read	= bin_attr_nvmem_read,
> -};
> -
> -static struct bin_attribute *nvmem_bin_ro_root_attributes[] = {
> -	&bin_attr_ro_root_nvmem,
> -	NULL,
> -};
> -
> -static const struct attribute_group nvmem_bin_ro_root_group = {
> -	.bin_attrs	= nvmem_bin_ro_root_attributes,
> -	.attrs		= nvmem_attrs,
> -};
> -
> -static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
> -	&nvmem_bin_ro_root_group,
> -	NULL,
> -};
> -
>   static void nvmem_release(struct device *dev)
>   {
>   	struct nvmem_device *nvmem = to_nvmem_device(dev);
> @@ -422,43 +210,6 @@ static int nvmem_add_cells(struct nvmem_device *nvmem,
>   	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_notifier() - Register a notifier block for nvmem events.
>    *
> @@ -651,14 +402,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
>   	nvmem->read_only = device_property_present(config->dev, "read-only") ||
>   			   config->read_only || !nvmem->reg_write;
>   
> -	if (config->root_only)
> -		nvmem->dev.groups = nvmem->read_only ?
> -			nvmem_ro_root_dev_groups :
> -			nvmem_rw_root_dev_groups;
> -	else
> -		nvmem->dev.groups = nvmem->read_only ?
> -			nvmem_ro_dev_groups :
> -			nvmem_rw_dev_groups;
> +	nvmem->dev.groups = nvmem_sysfs_get_groups(nvmem, config);
>   
>   	device_initialize(&nvmem->dev);
>   
> @@ -669,7 +413,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
>   		goto err_put_device;
>   
>   	if (config->compat) {
> -		rval = nvmem_setup_compat(nvmem, config);
> +		rval = nvmem_sysfs_setup_compat(nvmem, config);
>   		if (rval)
>   			goto err_device_del;
>   	}
> @@ -696,7 +440,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
>   	nvmem_device_remove_all_cells(nvmem);
>   err_teardown_compat:
>   	if (config->compat)
> -		device_remove_bin_file(nvmem->base_dev, &nvmem->eeprom);
> +		nvmem_sysfs_remove_compat(nvmem, config);
>   err_device_del:
>   	device_del(&nvmem->dev);
>   err_put_device:
> diff --git a/drivers/nvmem/nvmem-sysfs.c b/drivers/nvmem/nvmem-sysfs.c
> new file mode 100644
> index 000000000000..6f303b91f6e7
> --- /dev/null
> +++ b/drivers/nvmem/nvmem-sysfs.c
> @@ -0,0 +1,256 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019, Linaro Limited
> + */
> +#include "nvmem.h"
> +
> +static const char * const nvmem_type_str[] = {
> +	[NVMEM_TYPE_UNKNOWN] = "Unknown",
> +	[NVMEM_TYPE_EEPROM] = "EEPROM",
> +	[NVMEM_TYPE_OTP] = "OTP",
> +	[NVMEM_TYPE_BATTERY_BACKED] = "Battery backed",
> +};
> +
> +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> +static struct lock_class_key eeprom_lock_key;
> +#endif
> +
> +static ssize_t type_show(struct device *dev,
> +			 struct device_attribute *attr, char *buf)
> +{
> +	struct nvmem_device *nvmem = to_nvmem_device(dev);
> +
> +	return sprintf(buf, "%s\n", nvmem_type_str[nvmem->type]);
> +}
> +
> +static DEVICE_ATTR_RO(type);
> +
> +static struct attribute *nvmem_attrs[] = {
> +	&dev_attr_type.attr,
> +	NULL,
> +};
> +
> +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;
> +	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;
> +
> +	if (count < nvmem->word_size)
> +		return -EINVAL;
> +
> +	if (pos + count > nvmem->size)
> +		count = nvmem->size - pos;
> +
> +	count = round_down(count, nvmem->word_size);
> +
> +	rc = nvmem->reg_read(nvmem->priv, pos, buf, count);
> +
> +	if (rc)
> +		return rc;
> +
> +	return count;
> +}
> +
> +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;
> +	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 -EFBIG;
> +
> +	if (count < nvmem->word_size)
> +		return -EINVAL;
> +
> +	if (pos + count > nvmem->size)
> +		count = nvmem->size - pos;
> +
> +	count = round_down(count, nvmem->word_size);
> +
> +	rc = nvmem->reg_write(nvmem->priv, pos, buf, count);
> +
> +	if (rc)
> +		return rc;
> +
> +	return count;
> +}
> +
> +/* default read/write permissions */
> +static struct bin_attribute bin_attr_rw_nvmem = {
> +	.attr	= {
> +		.name	= "nvmem",
> +		.mode	= 0644,
> +	},
> +	.read	= bin_attr_nvmem_read,
> +	.write	= bin_attr_nvmem_write,
> +};
> +
> +static struct bin_attribute *nvmem_bin_rw_attributes[] = {
> +	&bin_attr_rw_nvmem,
> +	NULL,
> +};
> +
> +static const struct attribute_group nvmem_bin_rw_group = {
> +	.bin_attrs	= nvmem_bin_rw_attributes,
> +	.attrs		= nvmem_attrs,
> +};
> +
> +static const struct attribute_group *nvmem_rw_dev_groups[] = {
> +	&nvmem_bin_rw_group,
> +	NULL,
> +};
> +
> +/* read only permission */
> +static struct bin_attribute bin_attr_ro_nvmem = {
> +	.attr	= {
> +		.name	= "nvmem",
> +		.mode	= 0444,
> +	},
> +	.read	= bin_attr_nvmem_read,
> +};
> +
> +static struct bin_attribute *nvmem_bin_ro_attributes[] = {
> +	&bin_attr_ro_nvmem,
> +	NULL,
> +};
> +
> +static const struct attribute_group nvmem_bin_ro_group = {
> +	.bin_attrs	= nvmem_bin_ro_attributes,
> +	.attrs		= nvmem_attrs,
> +};
> +
> +static const struct attribute_group *nvmem_ro_dev_groups[] = {
> +	&nvmem_bin_ro_group,
> +	NULL,
> +};
> +
> +/* default read/write permissions, root only */
> +static struct bin_attribute bin_attr_rw_root_nvmem = {
> +	.attr	= {
> +		.name	= "nvmem",
> +		.mode	= 0600,
> +	},
> +	.read	= bin_attr_nvmem_read,
> +	.write	= bin_attr_nvmem_write,
> +};
> +
> +static struct bin_attribute *nvmem_bin_rw_root_attributes[] = {
> +	&bin_attr_rw_root_nvmem,
> +	NULL,
> +};
> +
> +static const struct attribute_group nvmem_bin_rw_root_group = {
> +	.bin_attrs	= nvmem_bin_rw_root_attributes,
> +	.attrs		= nvmem_attrs,
> +};
> +
> +static const struct attribute_group *nvmem_rw_root_dev_groups[] = {
> +	&nvmem_bin_rw_root_group,
> +	NULL,
> +};
> +
> +/* read only permission, root only */
> +static struct bin_attribute bin_attr_ro_root_nvmem = {
> +	.attr	= {
> +		.name	= "nvmem",
> +		.mode	= 0400,
> +	},
> +	.read	= bin_attr_nvmem_read,
> +};
> +
> +static struct bin_attribute *nvmem_bin_ro_root_attributes[] = {
> +	&bin_attr_ro_root_nvmem,
> +	NULL,
> +};
> +
> +static const struct attribute_group nvmem_bin_ro_root_group = {
> +	.bin_attrs	= nvmem_bin_ro_root_attributes,
> +	.attrs		= nvmem_attrs,
> +};
> +
> +static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
> +	&nvmem_bin_ro_root_group,
> +	NULL,
> +};
> +
> +const struct attribute_group **nvmem_sysfs_get_groups(
> +					struct nvmem_device *nvmem,
> +					const struct nvmem_config *config)
> +{
> +	if (config->root_only)
> +		return nvmem->read_only ?
> +			nvmem_ro_root_dev_groups :
> +			nvmem_rw_root_dev_groups;
> +
> +	return nvmem->read_only ? nvmem_ro_dev_groups : nvmem_rw_dev_groups;
> +}
> +
> +/*
> + * nvmem_setup_compat() - Create an additional binary entry in
> + * drivers sys directory, to be backwards compatible with the older
> + * drivers/misc/eeprom drivers.
> + */
> +int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
> +			      const struct nvmem_config *config)
> +{
> +	int rval;
> +
> +	if (!config->compat)
> +		return 0;
> +
> +	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;
> +}
> +
> +void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem,
> +			      const struct nvmem_config *config)
> +{
> +	if (config->compat)
> +		device_remove_bin_file(nvmem->base_dev, &nvmem->eeprom);
> +}
> diff --git a/drivers/nvmem/nvmem.h b/drivers/nvmem/nvmem.h
> new file mode 100644
> index 000000000000..eb8ed7121fa3
> --- /dev/null
> +++ b/drivers/nvmem/nvmem.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _DRIVERS_NVMEM_H
> +#define _DRIVERS_NVMEM_H
> +
> +#include <linux/device.h>
> +#include <linux/fs.h>
> +#include <linux/kref.h>
> +#include <linux/list.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/nvmem-provider.h>
> +
> +struct nvmem_device {
> +	struct module		*owner;
> +	struct device		dev;
> +	int			stride;
> +	int			word_size;
> +	int			id;
> +	struct kref		refcnt;
> +	size_t			size;
> +	bool			read_only;
> +	int			flags;
> +	enum nvmem_type		type;
> +	struct bin_attribute	eeprom;
> +	struct device		*base_dev;
> +	struct list_head	cells;
> +	nvmem_reg_read_t	reg_read;
> +	nvmem_reg_write_t	reg_write;
> +	void *priv;
> +};
> +
> +#define to_nvmem_device(d) container_of(d, struct nvmem_device, dev)
> +#define FLAG_COMPAT		BIT(0)
> +
> +#ifdef CONFIG_NVMEM_SYSFS
> +const struct attribute_group **nvmem_sysfs_get_groups(
> +					struct nvmem_device *nvmem,
> +					const struct nvmem_config *config);
> +int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
> +			      const struct nvmem_config *config);
> +void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem,
> +			      const struct nvmem_config *config);
> +#else
> +static inline const struct attribute_group **nvmem_sysfs_get_groups(
> +					struct nvmem_device *nvmem,
> +					const struct nvmem_config *config)
> +{
> +	return NULL;
> +}
> +
> +static inline int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
> +				      const struct nvmem_config *config)
> +{
> +	return -ENOSYS;
> +}
> +static inline void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem,
> +			      const struct nvmem_config *config)
> +{
> +}
> +#endif /* CONFIG_NVMEM_SYSFS */
> +
> +#endif /* _DRIVERS_NVMEM_H */
> 

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

Re: [PATCH] nvmem: core: add NVMEM_SYSFS Kconfig

[Mika Westerberg]

On Tue, Apr 16, 2019 at 10:49:22AM +0530, Gaurav Kohli wrote:
> Hi Srinivas,
> 
> Thanks for the patch,
> By default NVMEM_SYSFS should be set true, those whose don't want they can
> disable the same.
> 
> If we go with disable option, there are chances of eeprom may break in below
> case:
> 
> if (config->compat) {
>                 rval = nvmem_sysfs_setup_compat(nvmem, config); -> this will
> return error as config is disabled.
>                 if (rval)
>                         goto err_device_del;
>         }

I also think this may cause problems with Thunderbolt devices because
the upgradeable NVM is exposed to the userspace via these sysfs files
and those are being used by fwupd. If the files disappear it makes NVM
upgrade somewhat harder ;-)

At least it would be good to include following as part of this series if
you plan to disable the sysfs entries by default:

diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig
index f4869c38c7e4..dd5facab0af2 100644
--- a/drivers/thunderbolt/Kconfig
+++ b/drivers/thunderbolt/Kconfig
@@ -7,6 +7,7 @@ menuconfig THUNDERBOLT
 	select CRYPTO
 	select CRYPTO_HASH
 	select NVMEM
+	select NVMEM_SYSFS
 	help
 	  Thunderbolt Controller driver. This driver is required if you
 	  want to hotplug Thunderbolt devices on Apple hardware or on PCs

Re: [PATCH] nvmem: core: add NVMEM_SYSFS Kconfig

[Srinivas Kandagatla]

On 16/04/2019 10:34, Mika Westerberg wrote:
> On Tue, Apr 16, 2019 at 10:49:22AM +0530, Gaurav Kohli wrote:
>> Hi Srinivas,
>>
>> Thanks for the patch,
>> By default NVMEM_SYSFS should be set true, those whose don't want they can
>> disable the same.
>>
>> If we go with disable option, there are chances of eeprom may break in below
>> case:
>>
>> if (config->compat) {
>>                  rval = nvmem_sysfs_setup_compat(nvmem, config); -> this will
>> return error as config is disabled.
>>                  if (rval)
>>                          goto err_device_del;
>>          }
> 
> I also think this may cause problems with Thunderbolt devices because
> the upgradeable NVM is exposed to the userspace via these sysfs files
> and those are being used by fwupd. If the files disappear it makes NVM
> upgrade somewhat harder ;-)
Thanks Mika and Kohli for input.

As Kohli suggested we could make NVMEM_SYSFS default to y so that the 
sysfs file is available by default like before and user/board specific 
can decide to deselect it.

Let me know if thats Okay with you, I can spin that in v2.

--srini

> 
> At least it would be good to include following as part of this series if
> you plan to disable the sysfs entries by default:
> 
> diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig
> index f4869c38c7e4..dd5facab0af2 100644
> --- a/drivers/thunderbolt/Kconfig
> +++ b/drivers/thunderbolt/Kconfig
> @@ -7,6 +7,7 @@ menuconfig THUNDERBOLT
>   	select CRYPTO
>   	select CRYPTO_HASH
>   	select NVMEM
> +	select NVMEM_SYSFS
>   	help
>   	  Thunderbolt Controller driver. This driver is required if you
>   	  want to hotplug Thunderbolt devices on Apple hardware or on PCs
> 

Re: [PATCH] nvmem: core: add NVMEM_SYSFS Kconfig

[Mika Westerberg]

On Tue, Apr 16, 2019 at 10:44:01AM +0100, Srinivas Kandagatla wrote:
> As Kohli suggested we could make NVMEM_SYSFS default to y so that the sysfs
> file is available by default like before and user/board specific can decide
> to deselect it.
> 
> Let me know if thats Okay with you, I can spin that in v2.

Yup, that works for me. Thanks!

[PATCH] arm64: Skip apply SSBS call for non SSBS system

[Gaurav Kohli]

In a system where no cpu's implement SSBS, for
them no need to set pstate. This might help to save
few cpu cycles during context switch.

Signed-off-by: Gaurav Kohli <gkohli@codeaurora.org>

diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 6089638..79f80f1 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -477,6 +477,13 @@ static void ssbs_thread_switch(struct task_struct *next)
 	struct pt_regs *regs = task_pt_regs(next);
 
 	/*
+	 * For Targets which don't have SSBS support, they
+	 * can return from here.
+	 */
+	if (!IS_ENABLED(CONFIG_ARM64_SSBD))
+		return;
+
+	/*
 	 * Nothing to do for kernel threads, but 'regs' may be junk
 	 * (e.g. idle task) so check the flags and bail early.
 	 */
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project

[PATCH] arm64: Skip apply SSBS call for non SSBS system

[Gaurav Kohli]

In a system where no cpu's implement SSBS, for
them no need to set pstate. This might help to save
few cpu cycles during context switch.

Signed-off-by: Gaurav Kohli <gkohli@codeaurora.org>

diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 6089638..79f80f1 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -477,6 +477,13 @@ static void ssbs_thread_switch(struct task_struct *next)
 	struct pt_regs *regs = task_pt_regs(next);
 
 	/*
+	 * For Targets which don't have SSBS support, they
+	 * can return from here.
+	 */
+	if (!IS_ENABLED(CONFIG_ARM64_SSBD))
+		return;
+
+	/*
 	 * Nothing to do for kernel threads, but 'regs' may be junk
 	 * (e.g. idle task) so check the flags and bail early.
 	 */
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] nvmem: core: add support to NVMEM_NO_SYSFS_ENTRY

[Gaurav Kohli]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Some users might not want to expose nvmem entry to sysfs and
only intend to use kernel interface so add such provision.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
  Documentation/ABI/stable/sysfs-bus-nvmem |  2 ++
  drivers/nvmem/Kconfig                    |  5 +++++
  drivers/nvmem/core.c                     | 11 ++++++-----
  3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/Documentation/ABI/stable/sysfs-bus-nvmem 
b/Documentation/ABI/stable/sysfs-bus-nvmem
index 5923ab4620c5..12aab0a85fea 100644
--- a/Documentation/ABI/stable/sysfs-bus-nvmem
+++ b/Documentation/ABI/stable/sysfs-bus-nvmem
@@ -6,6 +6,8 @@ Description:
  		This file allows user to read/write the raw NVMEM contents.
  		Permissions for write to this file depends on the nvmem
  		provider configuration.
+		Note: This file is not present if CONFIG_NVMEM_NO_SYSFS_ENTRY
+		is enabled

  		ex:
  		hexdump /sys/bus/nvmem/devices/qfprom0/nvmem
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index 0a7a470ee859..6ab3276d287c 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -192,4 +192,9 @@ config SC27XX_EFUSE
  	  This driver can also be built as a module. If so, the module
  	  will be called nvmem-sc27xx-efuse.

+config NVMEM_NO_SYSFS_ENTRY
+	bool "No nvmem sysfs entry"
+
+	help
+	  Say Yes if you do not want to add nvmem entry to sysfs.
  endif
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index b9a0270883a0..c70f183fe379 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -216,7 +216,7 @@ static const struct attribute_group 
nvmem_bin_rw_group = {
	.attrs		= nvmem_attrs,
  };

-static const struct attribute_group *nvmem_rw_dev_groups[] = {
+static const __maybe_unused struct attribute_group 
*nvmem_rw_dev_groups[] = {
  	&nvmem_bin_rw_group,
  	NULL,
  };
@@ -240,7 +240,7 @@ static const struct attribute_group 
nvmem_bin_ro_group = {
  	.attrs		= nvmem_attrs,
  };

-static const struct attribute_group *nvmem_ro_dev_groups[] = {
+static const __maybe_unused struct attribute_group 
*nvmem_ro_dev_groups[] = {
  	&nvmem_bin_ro_group,
  	NULL,
  };
@@ -265,7 +265,7 @@ static const struct attribute_group 
nvmem_bin_rw_root_group = {
  	.attrs		= nvmem_attrs,
  };

-static const struct attribute_group *nvmem_rw_root_dev_groups[] = {
+static const __maybe_unused struct attribute_group 
*nvmem_rw_root_dev_groups[] = {
  	&nvmem_bin_rw_root_group,
  	NULL,
  };
@@ -289,7 +289,7 @@ static const struct attribute_group 
nvmem_bin_ro_root_group = {
  	.attrs		= nvmem_attrs,
  };

-static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
+static const __maybe_unused struct attribute_group 
*nvmem_ro_root_dev_groups[] = {
  	&nvmem_bin_ro_root_group,
  	NULL,
  };
@@ -688,6 +688,7 @@ struct nvmem_device *nvmem_register(const struct 
nvmem_config *config)
  	nvmem->read_only = device_property_present(config->dev, "read-only") |
  			   config->read_only;

+#if !defined(CONFIG_NVMEM_NO_SYSFS_ENTRY)
  	if (config->root_only)
  		nvmem->dev.groups = nvmem->read_only ?
  			nvmem_ro_root_dev_groups :
@@ -696,7 +697,7 @@ struct nvmem_device *nvmem_register(const struct 
nvmem_config *config)
  		nvmem->dev.groups = nvmem->read_only ?
  			nvmem_ro_dev_groups :
  			nvmem_rw_dev_groups;
-
+#endif
  	device_initialize(&nvmem->dev);

  	dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name);
-- 
2.21.0

[PATCH] nvmem: core: add support to NVMEM_NO_SYSFS_ENTRY

[Gaurav Kohli]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Some users might not want to expose nvmem entry to sysfs and
only intend to use kernel interface so add such provision.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
  Documentation/ABI/stable/sysfs-bus-nvmem |  2 ++
  drivers/nvmem/Kconfig                    |  5 +++++
  drivers/nvmem/core.c                     | 11 ++++++-----
  3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/Documentation/ABI/stable/sysfs-bus-nvmem 
b/Documentation/ABI/stable/sysfs-bus-nvmem
index 5923ab4620c5..12aab0a85fea 100644
--- a/Documentation/ABI/stable/sysfs-bus-nvmem
+++ b/Documentation/ABI/stable/sysfs-bus-nvmem
@@ -6,6 +6,8 @@ Description:
  		This file allows user to read/write the raw NVMEM contents.
  		Permissions for write to this file depends on the nvmem
  		provider configuration.
+		Note: This file is not present if CONFIG_NVMEM_NO_SYSFS_ENTRY
+		is enabled

  		ex:
  		hexdump /sys/bus/nvmem/devices/qfprom0/nvmem
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index 0a7a470ee859..6ab3276d287c 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -192,4 +192,9 @@ config SC27XX_EFUSE
  	  This driver can also be built as a module. If so, the module
  	  will be called nvmem-sc27xx-efuse.

+config NVMEM_NO_SYSFS_ENTRY
+	bool "No nvmem sysfs entry"
+
+	help
+	  Say Yes if you do not want to add nvmem entry to sysfs.
  endif
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index b9a0270883a0..c70f183fe379 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -216,7 +216,7 @@ static const struct attribute_group 
nvmem_bin_rw_group = {
	.attrs		= nvmem_attrs,
  };

-static const struct attribute_group *nvmem_rw_dev_groups[] = {
+static const __maybe_unused struct attribute_group 
*nvmem_rw_dev_groups[] = {
  	&nvmem_bin_rw_group,
  	NULL,
  };
@@ -240,7 +240,7 @@ static const struct attribute_group 
nvmem_bin_ro_group = {
  	.attrs		= nvmem_attrs,
  };

-static const struct attribute_group *nvmem_ro_dev_groups[] = {
+static const __maybe_unused struct attribute_group 
*nvmem_ro_dev_groups[] = {
  	&nvmem_bin_ro_group,
  	NULL,
  };
@@ -265,7 +265,7 @@ static const struct attribute_group 
nvmem_bin_rw_root_group = {
  	.attrs		= nvmem_attrs,
  };

-static const struct attribute_group *nvmem_rw_root_dev_groups[] = {
+static const __maybe_unused struct attribute_group 
*nvmem_rw_root_dev_groups[] = {
  	&nvmem_bin_rw_root_group,
  	NULL,
  };
@@ -289,7 +289,7 @@ static const struct attribute_group 
nvmem_bin_ro_root_group = {
  	.attrs		= nvmem_attrs,
  };

-static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
+static const __maybe_unused struct attribute_group 
*nvmem_ro_root_dev_groups[] = {
  	&nvmem_bin_ro_root_group,
  	NULL,
  };
@@ -688,6 +688,7 @@ struct nvmem_device *nvmem_register(const struct 
nvmem_config *config)
  	nvmem->read_only = device_property_present(config->dev, "read-only") |
  			   config->read_only;

+#if !defined(CONFIG_NVMEM_NO_SYSFS_ENTRY)
  	if (config->root_only)
  		nvmem->dev.groups = nvmem->read_only ?
  			nvmem_ro_root_dev_groups :
@@ -696,7 +697,7 @@ struct nvmem_device *nvmem_register(const struct 
nvmem_config *config)
  		nvmem->dev.groups = nvmem->read_only ?
  			nvmem_ro_dev_groups :
  			nvmem_rw_dev_groups;
-
+#endif
  	device_initialize(&nvmem->dev);

  	dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name);
-- 
2.21.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] nvmem: core: add NVMEM_SYSFS Kconfig

[Gaurav Kohli]

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Many nvmem providers are not very keen on having default sysfs
nvmem entry, as most of the usecases for them are inside kernel
itself. And in some cases read/writes to some areas in nvmem are
restricted and trapped at secure monitor level, so accessing them
from userspace would result in board reboots.

This patch adds new NVMEM_SYSFS Kconfig to make binary sysfs entry
an optional one. This provision will give more flexibility to users.
This patch also moves existing sysfs code to a new file so that its
not compiled in when its not really required.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 Documentation/ABI/stable/sysfs-bus-nvmem |   2 +
 drivers/nvmem/Kconfig                    |   9 +
 drivers/nvmem/Makefile                   |   3 +
 drivers/nvmem/core.c                     | 264 +----------------------
 drivers/nvmem/nvmem-sysfs.c              | 256 ++++++++++++++++++++++
 drivers/nvmem/nvmem.h                    |  62 ++++++
 6 files changed, 336 insertions(+), 260 deletions(-)
 create mode 100644 drivers/nvmem/nvmem-sysfs.c
 create mode 100644 drivers/nvmem/nvmem.h

diff --git a/Documentation/ABI/stable/sysfs-bus-nvmem b/Documentation/ABI/stable/sysfs-bus-nvmem
index 5923ab4620c5..9ffba8576f7b 100644
--- a/Documentation/ABI/stable/sysfs-bus-nvmem
+++ b/Documentation/ABI/stable/sysfs-bus-nvmem
@@ -6,6 +6,8 @@ Description:
 		This file allows user to read/write the raw NVMEM contents.
 		Permissions for write to this file depends on the nvmem
 		provider configuration.
+		Note: This file is only present if CONFIG_NVMEM_SYSFS
+		is enabled
 
 		ex:
 		hexdump /sys/bus/nvmem/devices/qfprom0/nvmem
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index a90e9a1ebe55..e316811e9c04 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -13,6 +13,15 @@ menuconfig NVMEM
 
 if NVMEM
 
+config NVMEM_SYSFS
+	bool "/sys/bus/nvmem/devices/*/nvmem (sysfs interface)"
+	depends on SYSFS
+	help
+	 Say Y here to add a sysfs interface for NVMEM.
+
+	 This interface is mostly used by userspace applications to
+	 read/write directly into nvmem.
+
 config NVMEM_IMX_IIM
 	tristate "i.MX IC Identification Module support"
 	depends on ARCH_MXC || COMPILE_TEST
diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
index 4c7ba12a7005..c1fe4768dfef 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -6,6 +6,9 @@
 obj-$(CONFIG_NVMEM)		+= nvmem_core.o
 nvmem_core-y			:= core.o
 
+obj-$(CONFIG_NVMEM_SYSFS)	+= nvmem_sysfs.o
+nvmem_sysfs-y			:= nvmem-sysfs.o
+
 # Devices
 obj-$(CONFIG_NVMEM_BCM_OCOTP)	+= nvmem-bcm-ocotp.o
 nvmem-bcm-ocotp-y		:= bcm-ocotp.o
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 5abebf2128b8..c7892c3da91f 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -17,27 +17,7 @@
 #include <linux/nvmem-provider.h>
 #include <linux/of.h>
 #include <linux/slab.h>
-
-struct nvmem_device {
-	struct module		*owner;
-	struct device		dev;
-	int			stride;
-	int			word_size;
-	int			id;
-	struct kref		refcnt;
-	size_t			size;
-	bool			read_only;
-	int			flags;
-	enum nvmem_type		type;
-	struct bin_attribute	eeprom;
-	struct device		*base_dev;
-	struct list_head	cells;
-	nvmem_reg_read_t	reg_read;
-	nvmem_reg_write_t	reg_write;
-	void *priv;
-};
-
-#define FLAG_COMPAT		BIT(0)
+#include "nvmem.h"
 
 struct nvmem_cell {
 	const char		*name;
@@ -61,18 +41,7 @@ static LIST_HEAD(nvmem_lookup_list);
 
 static BLOCKING_NOTIFIER_HEAD(nvmem_notifier);
 
-static const char * const nvmem_type_str[] = {
-	[NVMEM_TYPE_UNKNOWN] = "Unknown",
-	[NVMEM_TYPE_EEPROM] = "EEPROM",
-	[NVMEM_TYPE_OTP] = "OTP",
-	[NVMEM_TYPE_BATTERY_BACKED] = "Battery backed",
-};
-
-#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 int nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
 			  void *val, size_t bytes)
 {
@@ -91,187 +60,6 @@ static int nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset,
 	return -EINVAL;
 }
 
-static ssize_t type_show(struct device *dev,
-			 struct device_attribute *attr, char *buf)
-{
-	struct nvmem_device *nvmem = to_nvmem_device(dev);
-
-	return sprintf(buf, "%s\n", nvmem_type_str[nvmem->type]);
-}
-
-static DEVICE_ATTR_RO(type);
-
-static struct attribute *nvmem_attrs[] = {
-	&dev_attr_type.attr,
-	NULL,
-};
-
-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;
-	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;
-
-	if (count < nvmem->word_size)
-		return -EINVAL;
-
-	if (pos + count > nvmem->size)
-		count = nvmem->size - pos;
-
-	count = round_down(count, nvmem->word_size);
-
-	rc = nvmem_reg_read(nvmem, pos, buf, count);
-
-	if (rc)
-		return rc;
-
-	return count;
-}
-
-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;
-	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 -EFBIG;
-
-	if (count < nvmem->word_size)
-		return -EINVAL;
-
-	if (pos + count > nvmem->size)
-		count = nvmem->size - pos;
-
-	count = round_down(count, nvmem->word_size);
-
-	rc = nvmem_reg_write(nvmem, pos, buf, count);
-
-	if (rc)
-		return rc;
-
-	return count;
-}
-
-/* default read/write permissions */
-static struct bin_attribute bin_attr_rw_nvmem = {
-	.attr	= {
-		.name	= "nvmem",
-		.mode	= 0644,
-	},
-	.read	= bin_attr_nvmem_read,
-	.write	= bin_attr_nvmem_write,
-};
-
-static struct bin_attribute *nvmem_bin_rw_attributes[] = {
-	&bin_attr_rw_nvmem,
-	NULL,
-};
-
-static const struct attribute_group nvmem_bin_rw_group = {
-	.bin_attrs	= nvmem_bin_rw_attributes,
-	.attrs		= nvmem_attrs,
-};
-
-static const struct attribute_group *nvmem_rw_dev_groups[] = {
-	&nvmem_bin_rw_group,
-	NULL,
-};
-
-/* read only permission */
-static struct bin_attribute bin_attr_ro_nvmem = {
-	.attr	= {
-		.name	= "nvmem",
-		.mode	= 0444,
-	},
-	.read	= bin_attr_nvmem_read,
-};
-
-static struct bin_attribute *nvmem_bin_ro_attributes[] = {
-	&bin_attr_ro_nvmem,
-	NULL,
-};
-
-static const struct attribute_group nvmem_bin_ro_group = {
-	.bin_attrs	= nvmem_bin_ro_attributes,
-	.attrs		= nvmem_attrs,
-};
-
-static const struct attribute_group *nvmem_ro_dev_groups[] = {
-	&nvmem_bin_ro_group,
-	NULL,
-};
-
-/* default read/write permissions, root only */
-static struct bin_attribute bin_attr_rw_root_nvmem = {
-	.attr	= {
-		.name	= "nvmem",
-		.mode	= 0600,
-	},
-	.read	= bin_attr_nvmem_read,
-	.write	= bin_attr_nvmem_write,
-};
-
-static struct bin_attribute *nvmem_bin_rw_root_attributes[] = {
-	&bin_attr_rw_root_nvmem,
-	NULL,
-};
-
-static const struct attribute_group nvmem_bin_rw_root_group = {
-	.bin_attrs	= nvmem_bin_rw_root_attributes,
-	.attrs		= nvmem_attrs,
-};
-
-static const struct attribute_group *nvmem_rw_root_dev_groups[] = {
-	&nvmem_bin_rw_root_group,
-	NULL,
-};
-
-/* read only permission, root only */
-static struct bin_attribute bin_attr_ro_root_nvmem = {
-	.attr	= {
-		.name	= "nvmem",
-		.mode	= 0400,
-	},
-	.read	= bin_attr_nvmem_read,
-};
-
-static struct bin_attribute *nvmem_bin_ro_root_attributes[] = {
-	&bin_attr_ro_root_nvmem,
-	NULL,
-};
-
-static const struct attribute_group nvmem_bin_ro_root_group = {
-	.bin_attrs	= nvmem_bin_ro_root_attributes,
-	.attrs		= nvmem_attrs,
-};
-
-static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
-	&nvmem_bin_ro_root_group,
-	NULL,
-};
-
 static void nvmem_release(struct device *dev)
 {
 	struct nvmem_device *nvmem = to_nvmem_device(dev);
@@ -422,43 +210,6 @@ static int nvmem_add_cells(struct nvmem_device *nvmem,
 	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_notifier() - Register a notifier block for nvmem events.
  *
@@ -651,14 +402,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 	nvmem->read_only = device_property_present(config->dev, "read-only") ||
 			   config->read_only || !nvmem->reg_write;
 
-	if (config->root_only)
-		nvmem->dev.groups = nvmem->read_only ?
-			nvmem_ro_root_dev_groups :
-			nvmem_rw_root_dev_groups;
-	else
-		nvmem->dev.groups = nvmem->read_only ?
-			nvmem_ro_dev_groups :
-			nvmem_rw_dev_groups;
+	nvmem->dev.groups = nvmem_sysfs_get_groups(nvmem, config);
 
 	device_initialize(&nvmem->dev);
 
@@ -669,7 +413,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 		goto err_put_device;
 
 	if (config->compat) {
-		rval = nvmem_setup_compat(nvmem, config);
+		rval = nvmem_sysfs_setup_compat(nvmem, config);
 		if (rval)
 			goto err_device_del;
 	}
@@ -696,7 +440,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 	nvmem_device_remove_all_cells(nvmem);
 err_teardown_compat:
 	if (config->compat)
-		device_remove_bin_file(nvmem->base_dev, &nvmem->eeprom);
+		nvmem_sysfs_remove_compat(nvmem, config);
 err_device_del:
 	device_del(&nvmem->dev);
 err_put_device:
diff --git a/drivers/nvmem/nvmem-sysfs.c b/drivers/nvmem/nvmem-sysfs.c
new file mode 100644
index 000000000000..6f303b91f6e7
--- /dev/null
+++ b/drivers/nvmem/nvmem-sysfs.c
@@ -0,0 +1,256 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019, Linaro Limited
+ */
+#include "nvmem.h"
+
+static const char * const nvmem_type_str[] = {
+	[NVMEM_TYPE_UNKNOWN] = "Unknown",
+	[NVMEM_TYPE_EEPROM] = "EEPROM",
+	[NVMEM_TYPE_OTP] = "OTP",
+	[NVMEM_TYPE_BATTERY_BACKED] = "Battery backed",
+};
+
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+static struct lock_class_key eeprom_lock_key;
+#endif
+
+static ssize_t type_show(struct device *dev,
+			 struct device_attribute *attr, char *buf)
+{
+	struct nvmem_device *nvmem = to_nvmem_device(dev);
+
+	return sprintf(buf, "%s\n", nvmem_type_str[nvmem->type]);
+}
+
+static DEVICE_ATTR_RO(type);
+
+static struct attribute *nvmem_attrs[] = {
+	&dev_attr_type.attr,
+	NULL,
+};
+
+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;
+	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;
+
+	if (count < nvmem->word_size)
+		return -EINVAL;
+
+	if (pos + count > nvmem->size)
+		count = nvmem->size - pos;
+
+	count = round_down(count, nvmem->word_size);
+
+	rc = nvmem->reg_read(nvmem->priv, pos, buf, count);
+
+	if (rc)
+		return rc;
+
+	return count;
+}
+
+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;
+	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 -EFBIG;
+
+	if (count < nvmem->word_size)
+		return -EINVAL;
+
+	if (pos + count > nvmem->size)
+		count = nvmem->size - pos;
+
+	count = round_down(count, nvmem->word_size);
+
+	rc = nvmem->reg_write(nvmem->priv, pos, buf, count);
+
+	if (rc)
+		return rc;
+
+	return count;
+}
+
+/* default read/write permissions */
+static struct bin_attribute bin_attr_rw_nvmem = {
+	.attr	= {
+		.name	= "nvmem",
+		.mode	= 0644,
+	},
+	.read	= bin_attr_nvmem_read,
+	.write	= bin_attr_nvmem_write,
+};
+
+static struct bin_attribute *nvmem_bin_rw_attributes[] = {
+	&bin_attr_rw_nvmem,
+	NULL,
+};
+
+static const struct attribute_group nvmem_bin_rw_group = {
+	.bin_attrs	= nvmem_bin_rw_attributes,
+	.attrs		= nvmem_attrs,
+};
+
+static const struct attribute_group *nvmem_rw_dev_groups[] = {
+	&nvmem_bin_rw_group,
+	NULL,
+};
+
+/* read only permission */
+static struct bin_attribute bin_attr_ro_nvmem = {
+	.attr	= {
+		.name	= "nvmem",
+		.mode	= 0444,
+	},
+	.read	= bin_attr_nvmem_read,
+};
+
+static struct bin_attribute *nvmem_bin_ro_attributes[] = {
+	&bin_attr_ro_nvmem,
+	NULL,
+};
+
+static const struct attribute_group nvmem_bin_ro_group = {
+	.bin_attrs	= nvmem_bin_ro_attributes,
+	.attrs		= nvmem_attrs,
+};
+
+static const struct attribute_group *nvmem_ro_dev_groups[] = {
+	&nvmem_bin_ro_group,
+	NULL,
+};
+
+/* default read/write permissions, root only */
+static struct bin_attribute bin_attr_rw_root_nvmem = {
+	.attr	= {
+		.name	= "nvmem",
+		.mode	= 0600,
+	},
+	.read	= bin_attr_nvmem_read,
+	.write	= bin_attr_nvmem_write,
+};
+
+static struct bin_attribute *nvmem_bin_rw_root_attributes[] = {
+	&bin_attr_rw_root_nvmem,
+	NULL,
+};
+
+static const struct attribute_group nvmem_bin_rw_root_group = {
+	.bin_attrs	= nvmem_bin_rw_root_attributes,
+	.attrs		= nvmem_attrs,
+};
+
+static const struct attribute_group *nvmem_rw_root_dev_groups[] = {
+	&nvmem_bin_rw_root_group,
+	NULL,
+};
+
+/* read only permission, root only */
+static struct bin_attribute bin_attr_ro_root_nvmem = {
+	.attr	= {
+		.name	= "nvmem",
+		.mode	= 0400,
+	},
+	.read	= bin_attr_nvmem_read,
+};
+
+static struct bin_attribute *nvmem_bin_ro_root_attributes[] = {
+	&bin_attr_ro_root_nvmem,
+	NULL,
+};
+
+static const struct attribute_group nvmem_bin_ro_root_group = {
+	.bin_attrs	= nvmem_bin_ro_root_attributes,
+	.attrs		= nvmem_attrs,
+};
+
+static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
+	&nvmem_bin_ro_root_group,
+	NULL,
+};
+
+const struct attribute_group **nvmem_sysfs_get_groups(
+					struct nvmem_device *nvmem,
+					const struct nvmem_config *config)
+{
+	if (config->root_only)
+		return nvmem->read_only ?
+			nvmem_ro_root_dev_groups :
+			nvmem_rw_root_dev_groups;
+
+	return nvmem->read_only ? nvmem_ro_dev_groups : nvmem_rw_dev_groups;
+}
+
+/*
+ * nvmem_setup_compat() - Create an additional binary entry in
+ * drivers sys directory, to be backwards compatible with the older
+ * drivers/misc/eeprom drivers.
+ */
+int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
+			      const struct nvmem_config *config)
+{
+	int rval;
+
+	if (!config->compat)
+		return 0;
+
+	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;
+}
+
+void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem,
+			      const struct nvmem_config *config)
+{
+	if (config->compat)
+		device_remove_bin_file(nvmem->base_dev, &nvmem->eeprom);
+}
diff --git a/drivers/nvmem/nvmem.h b/drivers/nvmem/nvmem.h
new file mode 100644
index 000000000000..eb8ed7121fa3
--- /dev/null
+++ b/drivers/nvmem/nvmem.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _DRIVERS_NVMEM_H
+#define _DRIVERS_NVMEM_H
+
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/kref.h>
+#include <linux/list.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/nvmem-provider.h>
+
+struct nvmem_device {
+	struct module		*owner;
+	struct device		dev;
+	int			stride;
+	int			word_size;
+	int			id;
+	struct kref		refcnt;
+	size_t			size;
+	bool			read_only;
+	int			flags;
+	enum nvmem_type		type;
+	struct bin_attribute	eeprom;
+	struct device		*base_dev;
+	struct list_head	cells;
+	nvmem_reg_read_t	reg_read;
+	nvmem_reg_write_t	reg_write;
+	void *priv;
+};
+
+#define to_nvmem_device(d) container_of(d, struct nvmem_device, dev)
+#define FLAG_COMPAT		BIT(0)
+
+#ifdef CONFIG_NVMEM_SYSFS
+const struct attribute_group **nvmem_sysfs_get_groups(
+					struct nvmem_device *nvmem,
+					const struct nvmem_config *config);
+int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
+			      const struct nvmem_config *config);
+void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem,
+			      const struct nvmem_config *config);
+#else
+static inline const struct attribute_group **nvmem_sysfs_get_groups(
+					struct nvmem_device *nvmem,
+					const struct nvmem_config *config)
+{
+	return NULL;
+}
+
+static inline int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
+				      const struct nvmem_config *config)
+{
+	return -ENOSYS;
+}
+static inline void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem,
+			      const struct nvmem_config *config)
+{
+}
+#endif /* CONFIG_NVMEM_SYSFS */
+
+#endif /* _DRIVERS_NVMEM_H */

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] nvmem: core: add support to NVMEM_NO_SYSFS_ENTRY

[Gaurav Kohli]

Please ignore this patch, some problem in my git send mail.
Apologies for the spam.


On 8/4/2020 7:44 PM, Gaurav Kohli wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> 
> Some users might not want to expose nvmem entry to sysfs and
> only intend to use kernel interface so add such provision.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>    Documentation/ABI/stable/sysfs-bus-nvmem |  2 ++
>    drivers/nvmem/Kconfig                    |  5 +++++
>    drivers/nvmem/core.c                     | 11 ++++++-----
>    3 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/ABI/stable/sysfs-bus-nvmem
> b/Documentation/ABI/stable/sysfs-bus-nvmem
> index 5923ab4620c5..12aab0a85fea 100644
> --- a/Documentation/ABI/stable/sysfs-bus-nvmem
> +++ b/Documentation/ABI/stable/sysfs-bus-nvmem
> @@ -6,6 +6,8 @@ Description:
>    		This file allows user to read/write the raw NVMEM contents.
>    		Permissions for write to this file depends on the nvmem
>    		provider configuration.
> +		Note: This file is not present if CONFIG_NVMEM_NO_SYSFS_ENTRY
> +		is enabled
> 
>    		ex:
>    		hexdump /sys/bus/nvmem/devices/qfprom0/nvmem
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index 0a7a470ee859..6ab3276d287c 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -192,4 +192,9 @@ config SC27XX_EFUSE
>    	  This driver can also be built as a module. If so, the module
>    	  will be called nvmem-sc27xx-efuse.
> 
> +config NVMEM_NO_SYSFS_ENTRY
> +	bool "No nvmem sysfs entry"
> +
> +	help
> +	  Say Yes if you do not want to add nvmem entry to sysfs.
>    endif
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index b9a0270883a0..c70f183fe379 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -216,7 +216,7 @@ static const struct attribute_group
> nvmem_bin_rw_group = {
> 	.attrs		= nvmem_attrs,
>    };
> 
> -static const struct attribute_group *nvmem_rw_dev_groups[] = {
> +static const __maybe_unused struct attribute_group
> *nvmem_rw_dev_groups[] = {
>    	&nvmem_bin_rw_group,
>    	NULL,
>    };
> @@ -240,7 +240,7 @@ static const struct attribute_group
> nvmem_bin_ro_group = {
>    	.attrs		= nvmem_attrs,
>    };
> 
> -static const struct attribute_group *nvmem_ro_dev_groups[] = {
> +static const __maybe_unused struct attribute_group
> *nvmem_ro_dev_groups[] = {
>    	&nvmem_bin_ro_group,
>    	NULL,
>    };
> @@ -265,7 +265,7 @@ static const struct attribute_group
> nvmem_bin_rw_root_group = {
>    	.attrs		= nvmem_attrs,
>    };
> 
> -static const struct attribute_group *nvmem_rw_root_dev_groups[] = {
> +static const __maybe_unused struct attribute_group
> *nvmem_rw_root_dev_groups[] = {
>    	&nvmem_bin_rw_root_group,
>    	NULL,
>    };
> @@ -289,7 +289,7 @@ static const struct attribute_group
> nvmem_bin_ro_root_group = {
>    	.attrs		= nvmem_attrs,
>    };
> 
> -static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
> +static const __maybe_unused struct attribute_group
> *nvmem_ro_root_dev_groups[] = {
>    	&nvmem_bin_ro_root_group,
>    	NULL,
>    };
> @@ -688,6 +688,7 @@ struct nvmem_device *nvmem_register(const struct
> nvmem_config *config)
>    	nvmem->read_only = device_property_present(config->dev, "read-only") |
>    			   config->read_only;
> 
> +#if !defined(CONFIG_NVMEM_NO_SYSFS_ENTRY)
>    	if (config->root_only)
>    		nvmem->dev.groups = nvmem->read_only ?
>    			nvmem_ro_root_dev_groups :
> @@ -696,7 +697,7 @@ struct nvmem_device *nvmem_register(const struct
> nvmem_config *config)
>    		nvmem->dev.groups = nvmem->read_only ?
>    			nvmem_ro_dev_groups :
>    			nvmem_rw_dev_groups;
> -
> +#endif
>    	device_initialize(&nvmem->dev);
> 
>    	dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name);
> 

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

Re: [PATCH] nvmem: core: add support to NVMEM_NO_SYSFS_ENTRY

[Gaurav Kohli]

Please ignore this patch, some problem in my git send mail.
Apologies for the spam.


On 8/4/2020 7:44 PM, Gaurav Kohli wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> 
> Some users might not want to expose nvmem entry to sysfs and
> only intend to use kernel interface so add such provision.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>    Documentation/ABI/stable/sysfs-bus-nvmem |  2 ++
>    drivers/nvmem/Kconfig                    |  5 +++++
>    drivers/nvmem/core.c                     | 11 ++++++-----
>    3 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/ABI/stable/sysfs-bus-nvmem
> b/Documentation/ABI/stable/sysfs-bus-nvmem
> index 5923ab4620c5..12aab0a85fea 100644
> --- a/Documentation/ABI/stable/sysfs-bus-nvmem
> +++ b/Documentation/ABI/stable/sysfs-bus-nvmem
> @@ -6,6 +6,8 @@ Description:
>    		This file allows user to read/write the raw NVMEM contents.
>    		Permissions for write to this file depends on the nvmem
>    		provider configuration.
> +		Note: This file is not present if CONFIG_NVMEM_NO_SYSFS_ENTRY
> +		is enabled
> 
>    		ex:
>    		hexdump /sys/bus/nvmem/devices/qfprom0/nvmem
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index 0a7a470ee859..6ab3276d287c 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -192,4 +192,9 @@ config SC27XX_EFUSE
>    	  This driver can also be built as a module. If so, the module
>    	  will be called nvmem-sc27xx-efuse.
> 
> +config NVMEM_NO_SYSFS_ENTRY
> +	bool "No nvmem sysfs entry"
> +
> +	help
> +	  Say Yes if you do not want to add nvmem entry to sysfs.
>    endif
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index b9a0270883a0..c70f183fe379 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -216,7 +216,7 @@ static const struct attribute_group
> nvmem_bin_rw_group = {
> 	.attrs		= nvmem_attrs,
>    };
> 
> -static const struct attribute_group *nvmem_rw_dev_groups[] = {
> +static const __maybe_unused struct attribute_group
> *nvmem_rw_dev_groups[] = {
>    	&nvmem_bin_rw_group,
>    	NULL,
>    };
> @@ -240,7 +240,7 @@ static const struct attribute_group
> nvmem_bin_ro_group = {
>    	.attrs		= nvmem_attrs,
>    };
> 
> -static const struct attribute_group *nvmem_ro_dev_groups[] = {
> +static const __maybe_unused struct attribute_group
> *nvmem_ro_dev_groups[] = {
>    	&nvmem_bin_ro_group,
>    	NULL,
>    };
> @@ -265,7 +265,7 @@ static const struct attribute_group
> nvmem_bin_rw_root_group = {
>    	.attrs		= nvmem_attrs,
>    };
> 
> -static const struct attribute_group *nvmem_rw_root_dev_groups[] = {
> +static const __maybe_unused struct attribute_group
> *nvmem_rw_root_dev_groups[] = {
>    	&nvmem_bin_rw_root_group,
>    	NULL,
>    };
> @@ -289,7 +289,7 @@ static const struct attribute_group
> nvmem_bin_ro_root_group = {
>    	.attrs		= nvmem_attrs,
>    };
> 
> -static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
> +static const __maybe_unused struct attribute_group
> *nvmem_ro_root_dev_groups[] = {
>    	&nvmem_bin_ro_root_group,
>    	NULL,
>    };
> @@ -688,6 +688,7 @@ struct nvmem_device *nvmem_register(const struct
> nvmem_config *config)
>    	nvmem->read_only = device_property_present(config->dev, "read-only") |
>    			   config->read_only;
> 
> +#if !defined(CONFIG_NVMEM_NO_SYSFS_ENTRY)
>    	if (config->root_only)
>    		nvmem->dev.groups = nvmem->read_only ?
>    			nvmem_ro_root_dev_groups :
> @@ -696,7 +697,7 @@ struct nvmem_device *nvmem_register(const struct
> nvmem_config *config)
>    		nvmem->dev.groups = nvmem->read_only ?
>    			nvmem_ro_dev_groups :
>    			nvmem_rw_dev_groups;
> -
> +#endif
>    	device_initialize(&nvmem->dev);
> 
>    	dev_dbg(&nvmem->dev, "Registering nvmem device %s\n", config->name);
> 

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] nvmem: core: add NVMEM_SYSFS Kconfig

[Gaurav Kohli]

Please ignore this patch also, sent due to git send mistake.
Apologies for the spam.


On 8/4/2020 7:44 PM, Gaurav Kohli wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> 
> Many nvmem providers are not very keen on having default sysfs
> nvmem entry, as most of the usecases for them are inside kernel
> itself. And in some cases read/writes to some areas in nvmem are
> restricted and trapped at secure monitor level, so accessing them
> from userspace would result in board reboots.
> 
> This patch adds new NVMEM_SYSFS Kconfig to make binary sysfs entry
> an optional one. This provision will give more flexibility to users.
> This patch also moves existing sysfs code to a new file so that its
> not compiled in when its not really required.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>   Documentation/ABI/stable/sysfs-bus-nvmem |   2 +
>   drivers/nvmem/Kconfig                    |   9 +
>   drivers/nvmem/Makefile                   |   3 +
>   drivers/nvmem/core.c                     | 264 +----------------------
>   drivers/nvmem/nvmem-sysfs.c              | 256 ++++++++++++++++++++++
>   drivers/nvmem/nvmem.h                    |  62 ++++++
>   6 files changed, 336 insertions(+), 260 deletions(-)
>   create mode 100644 drivers/nvmem/nvmem-sysfs.c
>   create mode 100644 drivers/nvmem/nvmem.h
> 
> diff --git a/Documentation/ABI/stable/sysfs-bus-nvmem b/Documentation/ABI/stable/sysfs-bus-nvmem
> index 5923ab4620c5..9ffba8576f7b 100644
> --- a/Documentation/ABI/stable/sysfs-bus-nvmem
> +++ b/Documentation/ABI/stable/sysfs-bus-nvmem
> @@ -6,6 +6,8 @@ Description:
>   		This file allows user to read/write the raw NVMEM contents.
>   		Permissions for write to this file depends on the nvmem
>   		provider configuration.
> +		Note: This file is only present if CONFIG_NVMEM_SYSFS
> +		is enabled
>   
>   		ex:
>   		hexdump /sys/bus/nvmem/devices/qfprom0/nvmem
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index a90e9a1ebe55..e316811e9c04 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -13,6 +13,15 @@ menuconfig NVMEM
>   
>   if NVMEM
>   
> +config NVMEM_SYSFS
> +	bool "/sys/bus/nvmem/devices/*/nvmem (sysfs interface)"
> +	depends on SYSFS
> +	help
> +	 Say Y here to add a sysfs interface for NVMEM.
> +
> +	 This interface is mostly used by userspace applications to
> +	 read/write directly into nvmem.
> +
>   config NVMEM_IMX_IIM
>   	tristate "i.MX IC Identification Module support"
>   	depends on ARCH_MXC || COMPILE_TEST
> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
> index 4c7ba12a7005..c1fe4768dfef 100644
> --- a/drivers/nvmem/Makefile
> +++ b/drivers/nvmem/Makefile
> @@ -6,6 +6,9 @@
>   obj-$(CONFIG_NVMEM)		+= nvmem_core.o
>   nvmem_core-y			:= core.o
>   
> +obj-$(CONFIG_NVMEM_SYSFS)	+= nvmem_sysfs.o
> +nvmem_sysfs-y			:= nvmem-sysfs.o
> +
>   # Devices
>   obj-$(CONFIG_NVMEM_BCM_OCOTP)	+= nvmem-bcm-ocotp.o
>   nvmem-bcm-ocotp-y		:= bcm-ocotp.o
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 5abebf2128b8..c7892c3da91f 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -17,27 +17,7 @@
>   #include <linux/nvmem-provider.h>
>   #include <linux/of.h>
>   #include <linux/slab.h>
> -
> -struct nvmem_device {
> -	struct module		*owner;
> -	struct device		dev;
> -	int			stride;
> -	int			word_size;
> -	int			id;
> -	struct kref		refcnt;
> -	size_t			size;
> -	bool			read_only;
> -	int			flags;
> -	enum nvmem_type		type;
> -	struct bin_attribute	eeprom;
> -	struct device		*base_dev;
> -	struct list_head	cells;
> -	nvmem_reg_read_t	reg_read;
> -	nvmem_reg_write_t	reg_write;
> -	void *priv;
> -};
> -
> -#define FLAG_COMPAT		BIT(0)
> +#include "nvmem.h"
>   
>   struct nvmem_cell {
>   	const char		*name;
> @@ -61,18 +41,7 @@ static LIST_HEAD(nvmem_lookup_list);
>   
>   static BLOCKING_NOTIFIER_HEAD(nvmem_notifier);
>   
> -static const char * const nvmem_type_str[] = {
> -	[NVMEM_TYPE_UNKNOWN] = "Unknown",
> -	[NVMEM_TYPE_EEPROM] = "EEPROM",
> -	[NVMEM_TYPE_OTP] = "OTP",
> -	[NVMEM_TYPE_BATTERY_BACKED] = "Battery backed",
> -};
> -
> -#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 int nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
>   			  void *val, size_t bytes)
>   {
> @@ -91,187 +60,6 @@ static int nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset,
>   	return -EINVAL;
>   }
>   
> -static ssize_t type_show(struct device *dev,
> -			 struct device_attribute *attr, char *buf)
> -{
> -	struct nvmem_device *nvmem = to_nvmem_device(dev);
> -
> -	return sprintf(buf, "%s\n", nvmem_type_str[nvmem->type]);
> -}
> -
> -static DEVICE_ATTR_RO(type);
> -
> -static struct attribute *nvmem_attrs[] = {
> -	&dev_attr_type.attr,
> -	NULL,
> -};
> -
> -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;
> -	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;
> -
> -	if (count < nvmem->word_size)
> -		return -EINVAL;
> -
> -	if (pos + count > nvmem->size)
> -		count = nvmem->size - pos;
> -
> -	count = round_down(count, nvmem->word_size);
> -
> -	rc = nvmem_reg_read(nvmem, pos, buf, count);
> -
> -	if (rc)
> -		return rc;
> -
> -	return count;
> -}
> -
> -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;
> -	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 -EFBIG;
> -
> -	if (count < nvmem->word_size)
> -		return -EINVAL;
> -
> -	if (pos + count > nvmem->size)
> -		count = nvmem->size - pos;
> -
> -	count = round_down(count, nvmem->word_size);
> -
> -	rc = nvmem_reg_write(nvmem, pos, buf, count);
> -
> -	if (rc)
> -		return rc;
> -
> -	return count;
> -}
> -
> -/* default read/write permissions */
> -static struct bin_attribute bin_attr_rw_nvmem = {
> -	.attr	= {
> -		.name	= "nvmem",
> -		.mode	= 0644,
> -	},
> -	.read	= bin_attr_nvmem_read,
> -	.write	= bin_attr_nvmem_write,
> -};
> -
> -static struct bin_attribute *nvmem_bin_rw_attributes[] = {
> -	&bin_attr_rw_nvmem,
> -	NULL,
> -};
> -
> -static const struct attribute_group nvmem_bin_rw_group = {
> -	.bin_attrs	= nvmem_bin_rw_attributes,
> -	.attrs		= nvmem_attrs,
> -};
> -
> -static const struct attribute_group *nvmem_rw_dev_groups[] = {
> -	&nvmem_bin_rw_group,
> -	NULL,
> -};
> -
> -/* read only permission */
> -static struct bin_attribute bin_attr_ro_nvmem = {
> -	.attr	= {
> -		.name	= "nvmem",
> -		.mode	= 0444,
> -	},
> -	.read	= bin_attr_nvmem_read,
> -};
> -
> -static struct bin_attribute *nvmem_bin_ro_attributes[] = {
> -	&bin_attr_ro_nvmem,
> -	NULL,
> -};
> -
> -static const struct attribute_group nvmem_bin_ro_group = {
> -	.bin_attrs	= nvmem_bin_ro_attributes,
> -	.attrs		= nvmem_attrs,
> -};
> -
> -static const struct attribute_group *nvmem_ro_dev_groups[] = {
> -	&nvmem_bin_ro_group,
> -	NULL,
> -};
> -
> -/* default read/write permissions, root only */
> -static struct bin_attribute bin_attr_rw_root_nvmem = {
> -	.attr	= {
> -		.name	= "nvmem",
> -		.mode	= 0600,
> -	},
> -	.read	= bin_attr_nvmem_read,
> -	.write	= bin_attr_nvmem_write,
> -};
> -
> -static struct bin_attribute *nvmem_bin_rw_root_attributes[] = {
> -	&bin_attr_rw_root_nvmem,
> -	NULL,
> -};
> -
> -static const struct attribute_group nvmem_bin_rw_root_group = {
> -	.bin_attrs	= nvmem_bin_rw_root_attributes,
> -	.attrs		= nvmem_attrs,
> -};
> -
> -static const struct attribute_group *nvmem_rw_root_dev_groups[] = {
> -	&nvmem_bin_rw_root_group,
> -	NULL,
> -};
> -
> -/* read only permission, root only */
> -static struct bin_attribute bin_attr_ro_root_nvmem = {
> -	.attr	= {
> -		.name	= "nvmem",
> -		.mode	= 0400,
> -	},
> -	.read	= bin_attr_nvmem_read,
> -};
> -
> -static struct bin_attribute *nvmem_bin_ro_root_attributes[] = {
> -	&bin_attr_ro_root_nvmem,
> -	NULL,
> -};
> -
> -static const struct attribute_group nvmem_bin_ro_root_group = {
> -	.bin_attrs	= nvmem_bin_ro_root_attributes,
> -	.attrs		= nvmem_attrs,
> -};
> -
> -static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
> -	&nvmem_bin_ro_root_group,
> -	NULL,
> -};
> -
>   static void nvmem_release(struct device *dev)
>   {
>   	struct nvmem_device *nvmem = to_nvmem_device(dev);
> @@ -422,43 +210,6 @@ static int nvmem_add_cells(struct nvmem_device *nvmem,
>   	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_notifier() - Register a notifier block for nvmem events.
>    *
> @@ -651,14 +402,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
>   	nvmem->read_only = device_property_present(config->dev, "read-only") ||
>   			   config->read_only || !nvmem->reg_write;
>   
> -	if (config->root_only)
> -		nvmem->dev.groups = nvmem->read_only ?
> -			nvmem_ro_root_dev_groups :
> -			nvmem_rw_root_dev_groups;
> -	else
> -		nvmem->dev.groups = nvmem->read_only ?
> -			nvmem_ro_dev_groups :
> -			nvmem_rw_dev_groups;
> +	nvmem->dev.groups = nvmem_sysfs_get_groups(nvmem, config);
>   
>   	device_initialize(&nvmem->dev);
>   
> @@ -669,7 +413,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
>   		goto err_put_device;
>   
>   	if (config->compat) {
> -		rval = nvmem_setup_compat(nvmem, config);
> +		rval = nvmem_sysfs_setup_compat(nvmem, config);
>   		if (rval)
>   			goto err_device_del;
>   	}
> @@ -696,7 +440,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
>   	nvmem_device_remove_all_cells(nvmem);
>   err_teardown_compat:
>   	if (config->compat)
> -		device_remove_bin_file(nvmem->base_dev, &nvmem->eeprom);
> +		nvmem_sysfs_remove_compat(nvmem, config);
>   err_device_del:
>   	device_del(&nvmem->dev);
>   err_put_device:
> diff --git a/drivers/nvmem/nvmem-sysfs.c b/drivers/nvmem/nvmem-sysfs.c
> new file mode 100644
> index 000000000000..6f303b91f6e7
> --- /dev/null
> +++ b/drivers/nvmem/nvmem-sysfs.c
> @@ -0,0 +1,256 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019, Linaro Limited
> + */
> +#include "nvmem.h"
> +
> +static const char * const nvmem_type_str[] = {
> +	[NVMEM_TYPE_UNKNOWN] = "Unknown",
> +	[NVMEM_TYPE_EEPROM] = "EEPROM",
> +	[NVMEM_TYPE_OTP] = "OTP",
> +	[NVMEM_TYPE_BATTERY_BACKED] = "Battery backed",
> +};
> +
> +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> +static struct lock_class_key eeprom_lock_key;
> +#endif
> +
> +static ssize_t type_show(struct device *dev,
> +			 struct device_attribute *attr, char *buf)
> +{
> +	struct nvmem_device *nvmem = to_nvmem_device(dev);
> +
> +	return sprintf(buf, "%s\n", nvmem_type_str[nvmem->type]);
> +}
> +
> +static DEVICE_ATTR_RO(type);
> +
> +static struct attribute *nvmem_attrs[] = {
> +	&dev_attr_type.attr,
> +	NULL,
> +};
> +
> +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;
> +	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;
> +
> +	if (count < nvmem->word_size)
> +		return -EINVAL;
> +
> +	if (pos + count > nvmem->size)
> +		count = nvmem->size - pos;
> +
> +	count = round_down(count, nvmem->word_size);
> +
> +	rc = nvmem->reg_read(nvmem->priv, pos, buf, count);
> +
> +	if (rc)
> +		return rc;
> +
> +	return count;
> +}
> +
> +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;
> +	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 -EFBIG;
> +
> +	if (count < nvmem->word_size)
> +		return -EINVAL;
> +
> +	if (pos + count > nvmem->size)
> +		count = nvmem->size - pos;
> +
> +	count = round_down(count, nvmem->word_size);
> +
> +	rc = nvmem->reg_write(nvmem->priv, pos, buf, count);
> +
> +	if (rc)
> +		return rc;
> +
> +	return count;
> +}
> +
> +/* default read/write permissions */
> +static struct bin_attribute bin_attr_rw_nvmem = {
> +	.attr	= {
> +		.name	= "nvmem",
> +		.mode	= 0644,
> +	},
> +	.read	= bin_attr_nvmem_read,
> +	.write	= bin_attr_nvmem_write,
> +};
> +
> +static struct bin_attribute *nvmem_bin_rw_attributes[] = {
> +	&bin_attr_rw_nvmem,
> +	NULL,
> +};
> +
> +static const struct attribute_group nvmem_bin_rw_group = {
> +	.bin_attrs	= nvmem_bin_rw_attributes,
> +	.attrs		= nvmem_attrs,
> +};
> +
> +static const struct attribute_group *nvmem_rw_dev_groups[] = {
> +	&nvmem_bin_rw_group,
> +	NULL,
> +};
> +
> +/* read only permission */
> +static struct bin_attribute bin_attr_ro_nvmem = {
> +	.attr	= {
> +		.name	= "nvmem",
> +		.mode	= 0444,
> +	},
> +	.read	= bin_attr_nvmem_read,
> +};
> +
> +static struct bin_attribute *nvmem_bin_ro_attributes[] = {
> +	&bin_attr_ro_nvmem,
> +	NULL,
> +};
> +
> +static const struct attribute_group nvmem_bin_ro_group = {
> +	.bin_attrs	= nvmem_bin_ro_attributes,
> +	.attrs		= nvmem_attrs,
> +};
> +
> +static const struct attribute_group *nvmem_ro_dev_groups[] = {
> +	&nvmem_bin_ro_group,
> +	NULL,
> +};
> +
> +/* default read/write permissions, root only */
> +static struct bin_attribute bin_attr_rw_root_nvmem = {
> +	.attr	= {
> +		.name	= "nvmem",
> +		.mode	= 0600,
> +	},
> +	.read	= bin_attr_nvmem_read,
> +	.write	= bin_attr_nvmem_write,
> +};
> +
> +static struct bin_attribute *nvmem_bin_rw_root_attributes[] = {
> +	&bin_attr_rw_root_nvmem,
> +	NULL,
> +};
> +
> +static const struct attribute_group nvmem_bin_rw_root_group = {
> +	.bin_attrs	= nvmem_bin_rw_root_attributes,
> +	.attrs		= nvmem_attrs,
> +};
> +
> +static const struct attribute_group *nvmem_rw_root_dev_groups[] = {
> +	&nvmem_bin_rw_root_group,
> +	NULL,
> +};
> +
> +/* read only permission, root only */
> +static struct bin_attribute bin_attr_ro_root_nvmem = {
> +	.attr	= {
> +		.name	= "nvmem",
> +		.mode	= 0400,
> +	},
> +	.read	= bin_attr_nvmem_read,
> +};
> +
> +static struct bin_attribute *nvmem_bin_ro_root_attributes[] = {
> +	&bin_attr_ro_root_nvmem,
> +	NULL,
> +};
> +
> +static const struct attribute_group nvmem_bin_ro_root_group = {
> +	.bin_attrs	= nvmem_bin_ro_root_attributes,
> +	.attrs		= nvmem_attrs,
> +};
> +
> +static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
> +	&nvmem_bin_ro_root_group,
> +	NULL,
> +};
> +
> +const struct attribute_group **nvmem_sysfs_get_groups(
> +					struct nvmem_device *nvmem,
> +					const struct nvmem_config *config)
> +{
> +	if (config->root_only)
> +		return nvmem->read_only ?
> +			nvmem_ro_root_dev_groups :
> +			nvmem_rw_root_dev_groups;
> +
> +	return nvmem->read_only ? nvmem_ro_dev_groups : nvmem_rw_dev_groups;
> +}
> +
> +/*
> + * nvmem_setup_compat() - Create an additional binary entry in
> + * drivers sys directory, to be backwards compatible with the older
> + * drivers/misc/eeprom drivers.
> + */
> +int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
> +			      const struct nvmem_config *config)
> +{
> +	int rval;
> +
> +	if (!config->compat)
> +		return 0;
> +
> +	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;
> +}
> +
> +void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem,
> +			      const struct nvmem_config *config)
> +{
> +	if (config->compat)
> +		device_remove_bin_file(nvmem->base_dev, &nvmem->eeprom);
> +}
> diff --git a/drivers/nvmem/nvmem.h b/drivers/nvmem/nvmem.h
> new file mode 100644
> index 000000000000..eb8ed7121fa3
> --- /dev/null
> +++ b/drivers/nvmem/nvmem.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _DRIVERS_NVMEM_H
> +#define _DRIVERS_NVMEM_H
> +
> +#include <linux/device.h>
> +#include <linux/fs.h>
> +#include <linux/kref.h>
> +#include <linux/list.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/nvmem-provider.h>
> +
> +struct nvmem_device {
> +	struct module		*owner;
> +	struct device		dev;
> +	int			stride;
> +	int			word_size;
> +	int			id;
> +	struct kref		refcnt;
> +	size_t			size;
> +	bool			read_only;
> +	int			flags;
> +	enum nvmem_type		type;
> +	struct bin_attribute	eeprom;
> +	struct device		*base_dev;
> +	struct list_head	cells;
> +	nvmem_reg_read_t	reg_read;
> +	nvmem_reg_write_t	reg_write;
> +	void *priv;
> +};
> +
> +#define to_nvmem_device(d) container_of(d, struct nvmem_device, dev)
> +#define FLAG_COMPAT		BIT(0)
> +
> +#ifdef CONFIG_NVMEM_SYSFS
> +const struct attribute_group **nvmem_sysfs_get_groups(
> +					struct nvmem_device *nvmem,
> +					const struct nvmem_config *config);
> +int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
> +			      const struct nvmem_config *config);
> +void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem,
> +			      const struct nvmem_config *config);
> +#else
> +static inline const struct attribute_group **nvmem_sysfs_get_groups(
> +					struct nvmem_device *nvmem,
> +					const struct nvmem_config *config)
> +{
> +	return NULL;
> +}
> +
> +static inline int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
> +				      const struct nvmem_config *config)
> +{
> +	return -ENOSYS;
> +}
> +static inline void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem,
> +			      const struct nvmem_config *config)
> +{
> +}
> +#endif /* CONFIG_NVMEM_SYSFS */
> +
> +#endif /* _DRIVERS_NVMEM_H */
> 

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

Re: [PATCH] nvmem: core: add NVMEM_SYSFS Kconfig

[Gaurav Kohli]

Please ignore this patch also, sent due to git send mistake.
Apologies for the spam.


On 8/4/2020 7:44 PM, Gaurav Kohli wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> 
> Many nvmem providers are not very keen on having default sysfs
> nvmem entry, as most of the usecases for them are inside kernel
> itself. And in some cases read/writes to some areas in nvmem are
> restricted and trapped at secure monitor level, so accessing them
> from userspace would result in board reboots.
> 
> This patch adds new NVMEM_SYSFS Kconfig to make binary sysfs entry
> an optional one. This provision will give more flexibility to users.
> This patch also moves existing sysfs code to a new file so that its
> not compiled in when its not really required.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>   Documentation/ABI/stable/sysfs-bus-nvmem |   2 +
>   drivers/nvmem/Kconfig                    |   9 +
>   drivers/nvmem/Makefile                   |   3 +
>   drivers/nvmem/core.c                     | 264 +----------------------
>   drivers/nvmem/nvmem-sysfs.c              | 256 ++++++++++++++++++++++
>   drivers/nvmem/nvmem.h                    |  62 ++++++
>   6 files changed, 336 insertions(+), 260 deletions(-)
>   create mode 100644 drivers/nvmem/nvmem-sysfs.c
>   create mode 100644 drivers/nvmem/nvmem.h
> 
> diff --git a/Documentation/ABI/stable/sysfs-bus-nvmem b/Documentation/ABI/stable/sysfs-bus-nvmem
> index 5923ab4620c5..9ffba8576f7b 100644
> --- a/Documentation/ABI/stable/sysfs-bus-nvmem
> +++ b/Documentation/ABI/stable/sysfs-bus-nvmem
> @@ -6,6 +6,8 @@ Description:
>   		This file allows user to read/write the raw NVMEM contents.
>   		Permissions for write to this file depends on the nvmem
>   		provider configuration.
> +		Note: This file is only present if CONFIG_NVMEM_SYSFS
> +		is enabled
>   
>   		ex:
>   		hexdump /sys/bus/nvmem/devices/qfprom0/nvmem
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index a90e9a1ebe55..e316811e9c04 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -13,6 +13,15 @@ menuconfig NVMEM
>   
>   if NVMEM
>   
> +config NVMEM_SYSFS
> +	bool "/sys/bus/nvmem/devices/*/nvmem (sysfs interface)"
> +	depends on SYSFS
> +	help
> +	 Say Y here to add a sysfs interface for NVMEM.
> +
> +	 This interface is mostly used by userspace applications to
> +	 read/write directly into nvmem.
> +
>   config NVMEM_IMX_IIM
>   	tristate "i.MX IC Identification Module support"
>   	depends on ARCH_MXC || COMPILE_TEST
> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
> index 4c7ba12a7005..c1fe4768dfef 100644
> --- a/drivers/nvmem/Makefile
> +++ b/drivers/nvmem/Makefile
> @@ -6,6 +6,9 @@
>   obj-$(CONFIG_NVMEM)		+= nvmem_core.o
>   nvmem_core-y			:= core.o
>   
> +obj-$(CONFIG_NVMEM_SYSFS)	+= nvmem_sysfs.o
> +nvmem_sysfs-y			:= nvmem-sysfs.o
> +
>   # Devices
>   obj-$(CONFIG_NVMEM_BCM_OCOTP)	+= nvmem-bcm-ocotp.o
>   nvmem-bcm-ocotp-y		:= bcm-ocotp.o
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 5abebf2128b8..c7892c3da91f 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -17,27 +17,7 @@
>   #include <linux/nvmem-provider.h>
>   #include <linux/of.h>
>   #include <linux/slab.h>
> -
> -struct nvmem_device {
> -	struct module		*owner;
> -	struct device		dev;
> -	int			stride;
> -	int			word_size;
> -	int			id;
> -	struct kref		refcnt;
> -	size_t			size;
> -	bool			read_only;
> -	int			flags;
> -	enum nvmem_type		type;
> -	struct bin_attribute	eeprom;
> -	struct device		*base_dev;
> -	struct list_head	cells;
> -	nvmem_reg_read_t	reg_read;
> -	nvmem_reg_write_t	reg_write;
> -	void *priv;
> -};
> -
> -#define FLAG_COMPAT		BIT(0)
> +#include "nvmem.h"
>   
>   struct nvmem_cell {
>   	const char		*name;
> @@ -61,18 +41,7 @@ static LIST_HEAD(nvmem_lookup_list);
>   
>   static BLOCKING_NOTIFIER_HEAD(nvmem_notifier);
>   
> -static const char * const nvmem_type_str[] = {
> -	[NVMEM_TYPE_UNKNOWN] = "Unknown",
> -	[NVMEM_TYPE_EEPROM] = "EEPROM",
> -	[NVMEM_TYPE_OTP] = "OTP",
> -	[NVMEM_TYPE_BATTERY_BACKED] = "Battery backed",
> -};
> -
> -#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 int nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
>   			  void *val, size_t bytes)
>   {
> @@ -91,187 +60,6 @@ static int nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset,
>   	return -EINVAL;
>   }
>   
> -static ssize_t type_show(struct device *dev,
> -			 struct device_attribute *attr, char *buf)
> -{
> -	struct nvmem_device *nvmem = to_nvmem_device(dev);
> -
> -	return sprintf(buf, "%s\n", nvmem_type_str[nvmem->type]);
> -}
> -
> -static DEVICE_ATTR_RO(type);
> -
> -static struct attribute *nvmem_attrs[] = {
> -	&dev_attr_type.attr,
> -	NULL,
> -};
> -
> -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;
> -	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;
> -
> -	if (count < nvmem->word_size)
> -		return -EINVAL;
> -
> -	if (pos + count > nvmem->size)
> -		count = nvmem->size - pos;
> -
> -	count = round_down(count, nvmem->word_size);
> -
> -	rc = nvmem_reg_read(nvmem, pos, buf, count);
> -
> -	if (rc)
> -		return rc;
> -
> -	return count;
> -}
> -
> -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;
> -	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 -EFBIG;
> -
> -	if (count < nvmem->word_size)
> -		return -EINVAL;
> -
> -	if (pos + count > nvmem->size)
> -		count = nvmem->size - pos;
> -
> -	count = round_down(count, nvmem->word_size);
> -
> -	rc = nvmem_reg_write(nvmem, pos, buf, count);
> -
> -	if (rc)
> -		return rc;
> -
> -	return count;
> -}
> -
> -/* default read/write permissions */
> -static struct bin_attribute bin_attr_rw_nvmem = {
> -	.attr	= {
> -		.name	= "nvmem",
> -		.mode	= 0644,
> -	},
> -	.read	= bin_attr_nvmem_read,
> -	.write	= bin_attr_nvmem_write,
> -};
> -
> -static struct bin_attribute *nvmem_bin_rw_attributes[] = {
> -	&bin_attr_rw_nvmem,
> -	NULL,
> -};
> -
> -static const struct attribute_group nvmem_bin_rw_group = {
> -	.bin_attrs	= nvmem_bin_rw_attributes,
> -	.attrs		= nvmem_attrs,
> -};
> -
> -static const struct attribute_group *nvmem_rw_dev_groups[] = {
> -	&nvmem_bin_rw_group,
> -	NULL,
> -};
> -
> -/* read only permission */
> -static struct bin_attribute bin_attr_ro_nvmem = {
> -	.attr	= {
> -		.name	= "nvmem",
> -		.mode	= 0444,
> -	},
> -	.read	= bin_attr_nvmem_read,
> -};
> -
> -static struct bin_attribute *nvmem_bin_ro_attributes[] = {
> -	&bin_attr_ro_nvmem,
> -	NULL,
> -};
> -
> -static const struct attribute_group nvmem_bin_ro_group = {
> -	.bin_attrs	= nvmem_bin_ro_attributes,
> -	.attrs		= nvmem_attrs,
> -};
> -
> -static const struct attribute_group *nvmem_ro_dev_groups[] = {
> -	&nvmem_bin_ro_group,
> -	NULL,
> -};
> -
> -/* default read/write permissions, root only */
> -static struct bin_attribute bin_attr_rw_root_nvmem = {
> -	.attr	= {
> -		.name	= "nvmem",
> -		.mode	= 0600,
> -	},
> -	.read	= bin_attr_nvmem_read,
> -	.write	= bin_attr_nvmem_write,
> -};
> -
> -static struct bin_attribute *nvmem_bin_rw_root_attributes[] = {
> -	&bin_attr_rw_root_nvmem,
> -	NULL,
> -};
> -
> -static const struct attribute_group nvmem_bin_rw_root_group = {
> -	.bin_attrs	= nvmem_bin_rw_root_attributes,
> -	.attrs		= nvmem_attrs,
> -};
> -
> -static const struct attribute_group *nvmem_rw_root_dev_groups[] = {
> -	&nvmem_bin_rw_root_group,
> -	NULL,
> -};
> -
> -/* read only permission, root only */
> -static struct bin_attribute bin_attr_ro_root_nvmem = {
> -	.attr	= {
> -		.name	= "nvmem",
> -		.mode	= 0400,
> -	},
> -	.read	= bin_attr_nvmem_read,
> -};
> -
> -static struct bin_attribute *nvmem_bin_ro_root_attributes[] = {
> -	&bin_attr_ro_root_nvmem,
> -	NULL,
> -};
> -
> -static const struct attribute_group nvmem_bin_ro_root_group = {
> -	.bin_attrs	= nvmem_bin_ro_root_attributes,
> -	.attrs		= nvmem_attrs,
> -};
> -
> -static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
> -	&nvmem_bin_ro_root_group,
> -	NULL,
> -};
> -
>   static void nvmem_release(struct device *dev)
>   {
>   	struct nvmem_device *nvmem = to_nvmem_device(dev);
> @@ -422,43 +210,6 @@ static int nvmem_add_cells(struct nvmem_device *nvmem,
>   	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_notifier() - Register a notifier block for nvmem events.
>    *
> @@ -651,14 +402,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
>   	nvmem->read_only = device_property_present(config->dev, "read-only") ||
>   			   config->read_only || !nvmem->reg_write;
>   
> -	if (config->root_only)
> -		nvmem->dev.groups = nvmem->read_only ?
> -			nvmem_ro_root_dev_groups :
> -			nvmem_rw_root_dev_groups;
> -	else
> -		nvmem->dev.groups = nvmem->read_only ?
> -			nvmem_ro_dev_groups :
> -			nvmem_rw_dev_groups;
> +	nvmem->dev.groups = nvmem_sysfs_get_groups(nvmem, config);
>   
>   	device_initialize(&nvmem->dev);
>   
> @@ -669,7 +413,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
>   		goto err_put_device;
>   
>   	if (config->compat) {
> -		rval = nvmem_setup_compat(nvmem, config);
> +		rval = nvmem_sysfs_setup_compat(nvmem, config);
>   		if (rval)
>   			goto err_device_del;
>   	}
> @@ -696,7 +440,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
>   	nvmem_device_remove_all_cells(nvmem);
>   err_teardown_compat:
>   	if (config->compat)
> -		device_remove_bin_file(nvmem->base_dev, &nvmem->eeprom);
> +		nvmem_sysfs_remove_compat(nvmem, config);
>   err_device_del:
>   	device_del(&nvmem->dev);
>   err_put_device:
> diff --git a/drivers/nvmem/nvmem-sysfs.c b/drivers/nvmem/nvmem-sysfs.c
> new file mode 100644
> index 000000000000..6f303b91f6e7
> --- /dev/null
> +++ b/drivers/nvmem/nvmem-sysfs.c
> @@ -0,0 +1,256 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019, Linaro Limited
> + */
> +#include "nvmem.h"
> +
> +static const char * const nvmem_type_str[] = {
> +	[NVMEM_TYPE_UNKNOWN] = "Unknown",
> +	[NVMEM_TYPE_EEPROM] = "EEPROM",
> +	[NVMEM_TYPE_OTP] = "OTP",
> +	[NVMEM_TYPE_BATTERY_BACKED] = "Battery backed",
> +};
> +
> +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> +static struct lock_class_key eeprom_lock_key;
> +#endif
> +
> +static ssize_t type_show(struct device *dev,
> +			 struct device_attribute *attr, char *buf)
> +{
> +	struct nvmem_device *nvmem = to_nvmem_device(dev);
> +
> +	return sprintf(buf, "%s\n", nvmem_type_str[nvmem->type]);
> +}
> +
> +static DEVICE_ATTR_RO(type);
> +
> +static struct attribute *nvmem_attrs[] = {
> +	&dev_attr_type.attr,
> +	NULL,
> +};
> +
> +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;
> +	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;
> +
> +	if (count < nvmem->word_size)
> +		return -EINVAL;
> +
> +	if (pos + count > nvmem->size)
> +		count = nvmem->size - pos;
> +
> +	count = round_down(count, nvmem->word_size);
> +
> +	rc = nvmem->reg_read(nvmem->priv, pos, buf, count);
> +
> +	if (rc)
> +		return rc;
> +
> +	return count;
> +}
> +
> +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;
> +	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 -EFBIG;
> +
> +	if (count < nvmem->word_size)
> +		return -EINVAL;
> +
> +	if (pos + count > nvmem->size)
> +		count = nvmem->size - pos;
> +
> +	count = round_down(count, nvmem->word_size);
> +
> +	rc = nvmem->reg_write(nvmem->priv, pos, buf, count);
> +
> +	if (rc)
> +		return rc;
> +
> +	return count;
> +}
> +
> +/* default read/write permissions */
> +static struct bin_attribute bin_attr_rw_nvmem = {
> +	.attr	= {
> +		.name	= "nvmem",
> +		.mode	= 0644,
> +	},
> +	.read	= bin_attr_nvmem_read,
> +	.write	= bin_attr_nvmem_write,
> +};
> +
> +static struct bin_attribute *nvmem_bin_rw_attributes[] = {
> +	&bin_attr_rw_nvmem,
> +	NULL,
> +};
> +
> +static const struct attribute_group nvmem_bin_rw_group = {
> +	.bin_attrs	= nvmem_bin_rw_attributes,
> +	.attrs		= nvmem_attrs,
> +};
> +
> +static const struct attribute_group *nvmem_rw_dev_groups[] = {
> +	&nvmem_bin_rw_group,
> +	NULL,
> +};
> +
> +/* read only permission */
> +static struct bin_attribute bin_attr_ro_nvmem = {
> +	.attr	= {
> +		.name	= "nvmem",
> +		.mode	= 0444,
> +	},
> +	.read	= bin_attr_nvmem_read,
> +};
> +
> +static struct bin_attribute *nvmem_bin_ro_attributes[] = {
> +	&bin_attr_ro_nvmem,
> +	NULL,
> +};
> +
> +static const struct attribute_group nvmem_bin_ro_group = {
> +	.bin_attrs	= nvmem_bin_ro_attributes,
> +	.attrs		= nvmem_attrs,
> +};
> +
> +static const struct attribute_group *nvmem_ro_dev_groups[] = {
> +	&nvmem_bin_ro_group,
> +	NULL,
> +};
> +
> +/* default read/write permissions, root only */
> +static struct bin_attribute bin_attr_rw_root_nvmem = {
> +	.attr	= {
> +		.name	= "nvmem",
> +		.mode	= 0600,
> +	},
> +	.read	= bin_attr_nvmem_read,
> +	.write	= bin_attr_nvmem_write,
> +};
> +
> +static struct bin_attribute *nvmem_bin_rw_root_attributes[] = {
> +	&bin_attr_rw_root_nvmem,
> +	NULL,
> +};
> +
> +static const struct attribute_group nvmem_bin_rw_root_group = {
> +	.bin_attrs	= nvmem_bin_rw_root_attributes,
> +	.attrs		= nvmem_attrs,
> +};
> +
> +static const struct attribute_group *nvmem_rw_root_dev_groups[] = {
> +	&nvmem_bin_rw_root_group,
> +	NULL,
> +};
> +
> +/* read only permission, root only */
> +static struct bin_attribute bin_attr_ro_root_nvmem = {
> +	.attr	= {
> +		.name	= "nvmem",
> +		.mode	= 0400,
> +	},
> +	.read	= bin_attr_nvmem_read,
> +};
> +
> +static struct bin_attribute *nvmem_bin_ro_root_attributes[] = {
> +	&bin_attr_ro_root_nvmem,
> +	NULL,
> +};
> +
> +static const struct attribute_group nvmem_bin_ro_root_group = {
> +	.bin_attrs	= nvmem_bin_ro_root_attributes,
> +	.attrs		= nvmem_attrs,
> +};
> +
> +static const struct attribute_group *nvmem_ro_root_dev_groups[] = {
> +	&nvmem_bin_ro_root_group,
> +	NULL,
> +};
> +
> +const struct attribute_group **nvmem_sysfs_get_groups(
> +					struct nvmem_device *nvmem,
> +					const struct nvmem_config *config)
> +{
> +	if (config->root_only)
> +		return nvmem->read_only ?
> +			nvmem_ro_root_dev_groups :
> +			nvmem_rw_root_dev_groups;
> +
> +	return nvmem->read_only ? nvmem_ro_dev_groups : nvmem_rw_dev_groups;
> +}
> +
> +/*
> + * nvmem_setup_compat() - Create an additional binary entry in
> + * drivers sys directory, to be backwards compatible with the older
> + * drivers/misc/eeprom drivers.
> + */
> +int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
> +			      const struct nvmem_config *config)
> +{
> +	int rval;
> +
> +	if (!config->compat)
> +		return 0;
> +
> +	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;
> +}
> +
> +void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem,
> +			      const struct nvmem_config *config)
> +{
> +	if (config->compat)
> +		device_remove_bin_file(nvmem->base_dev, &nvmem->eeprom);
> +}
> diff --git a/drivers/nvmem/nvmem.h b/drivers/nvmem/nvmem.h
> new file mode 100644
> index 000000000000..eb8ed7121fa3
> --- /dev/null
> +++ b/drivers/nvmem/nvmem.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _DRIVERS_NVMEM_H
> +#define _DRIVERS_NVMEM_H
> +
> +#include <linux/device.h>
> +#include <linux/fs.h>
> +#include <linux/kref.h>
> +#include <linux/list.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/nvmem-provider.h>
> +
> +struct nvmem_device {
> +	struct module		*owner;
> +	struct device		dev;
> +	int			stride;
> +	int			word_size;
> +	int			id;
> +	struct kref		refcnt;
> +	size_t			size;
> +	bool			read_only;
> +	int			flags;
> +	enum nvmem_type		type;
> +	struct bin_attribute	eeprom;
> +	struct device		*base_dev;
> +	struct list_head	cells;
> +	nvmem_reg_read_t	reg_read;
> +	nvmem_reg_write_t	reg_write;
> +	void *priv;
> +};
> +
> +#define to_nvmem_device(d) container_of(d, struct nvmem_device, dev)
> +#define FLAG_COMPAT		BIT(0)
> +
> +#ifdef CONFIG_NVMEM_SYSFS
> +const struct attribute_group **nvmem_sysfs_get_groups(
> +					struct nvmem_device *nvmem,
> +					const struct nvmem_config *config);
> +int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
> +			      const struct nvmem_config *config);
> +void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem,
> +			      const struct nvmem_config *config);
> +#else
> +static inline const struct attribute_group **nvmem_sysfs_get_groups(
> +					struct nvmem_device *nvmem,
> +					const struct nvmem_config *config)
> +{
> +	return NULL;
> +}
> +
> +static inline int nvmem_sysfs_setup_compat(struct nvmem_device *nvmem,
> +				      const struct nvmem_config *config)
> +{
> +	return -ENOSYS;
> +}
> +static inline void nvmem_sysfs_remove_compat(struct nvmem_device *nvmem,
> +			      const struct nvmem_config *config)
> +{
> +}
> +#endif /* CONFIG_NVMEM_SYSFS */
> +
> +#endif /* _DRIVERS_NVMEM_H */
> 

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Skip apply SSBS call for non SSBS system

[Gaurav Kohli]

Hi,

Please let us know, is below patch good to have
or not for non ssbs systems.

On 8/4/2020 7:44 PM, Gaurav Kohli wrote:
> In a system where no cpu's implement SSBS, for
> them no need to set pstate. This might help to save
> few cpu cycles during context switch.
> 
> Signed-off-by: Gaurav Kohli <gkohli@codeaurora.org>
> 
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 6089638..79f80f1 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -477,6 +477,13 @@ static void ssbs_thread_switch(struct task_struct *next)
>   	struct pt_regs *regs = task_pt_regs(next);
>   
>   	/*
> +	 * For Targets which don't have SSBS support, they
> +	 * can return from here.
> +	 */
> +	if (!IS_ENABLED(CONFIG_ARM64_SSBD))
> +		return;
> +
> +	/*
>   	 * Nothing to do for kernel threads, but 'regs' may be junk
>   	 * (e.g. idle task) so check the flags and bail early.
>   	 */
> 

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

Re: [PATCH] arm64: Skip apply SSBS call for non SSBS system

[Gaurav Kohli]

Hi,

Please let us know, is below patch good to have
or not for non ssbs systems.

On 8/4/2020 7:44 PM, Gaurav Kohli wrote:
> In a system where no cpu's implement SSBS, for
> them no need to set pstate. This might help to save
> few cpu cycles during context switch.
> 
> Signed-off-by: Gaurav Kohli <gkohli@codeaurora.org>
> 
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 6089638..79f80f1 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -477,6 +477,13 @@ static void ssbs_thread_switch(struct task_struct *next)
>   	struct pt_regs *regs = task_pt_regs(next);
>   
>   	/*
> +	 * For Targets which don't have SSBS support, they
> +	 * can return from here.
> +	 */
> +	if (!IS_ENABLED(CONFIG_ARM64_SSBD))
> +		return;
> +
> +	/*
>   	 * Nothing to do for kernel threads, but 'regs' may be junk
>   	 * (e.g. idle task) so check the flags and bail early.
>   	 */
> 

-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Skip apply SSBS call for non SSBS system

[Will Deacon]

On Tue, Aug 04, 2020 at 07:44:42PM +0530, Gaurav Kohli wrote:
> In a system where no cpu's implement SSBS, for
> them no need to set pstate. This might help to save
> few cpu cycles during context switch.
> 
> Signed-off-by: Gaurav Kohli <gkohli@codeaurora.org>
> 
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 6089638..79f80f1 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -477,6 +477,13 @@ static void ssbs_thread_switch(struct task_struct *next)
>  	struct pt_regs *regs = task_pt_regs(next);
>  
>  	/*
> +	 * For Targets which don't have SSBS support, they
> +	 * can return from here.
> +	 */
> +	if (!IS_ENABLED(CONFIG_ARM64_SSBD))
> +		return;

Does this actually make a measurable difference?

Will

Re: [PATCH] arm64: Skip apply SSBS call for non SSBS system

[Will Deacon]

On Tue, Aug 04, 2020 at 07:44:42PM +0530, Gaurav Kohli wrote:
> In a system where no cpu's implement SSBS, for
> them no need to set pstate. This might help to save
> few cpu cycles during context switch.
> 
> Signed-off-by: Gaurav Kohli <gkohli@codeaurora.org>
> 
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 6089638..79f80f1 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -477,6 +477,13 @@ static void ssbs_thread_switch(struct task_struct *next)
>  	struct pt_regs *regs = task_pt_regs(next);
>  
>  	/*
> +	 * For Targets which don't have SSBS support, they
> +	 * can return from here.
> +	 */
> +	if (!IS_ENABLED(CONFIG_ARM64_SSBD))
> +		return;

Does this actually make a measurable difference?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: Skip apply SSBS call for non SSBS system

[Gaurav Kohli]

On 8/12/2020 7:00 PM, Will Deacon wrote:
> On Tue, Aug 04, 2020 at 07:44:42PM +0530, Gaurav Kohli wrote:
>> In a system where no cpu's implement SSBS, for
>> them no need to set pstate. This might help to save
>> few cpu cycles during context switch.
>>
>> Signed-off-by: Gaurav Kohli <gkohli@codeaurora.org>
>>
>> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
>> index 6089638..79f80f1 100644
>> --- a/arch/arm64/kernel/process.c
>> +++ b/arch/arm64/kernel/process.c
>> @@ -477,6 +477,13 @@ static void ssbs_thread_switch(struct task_struct *next)
>>   	struct pt_regs *regs = task_pt_regs(next);
>>   
>>   	/*
>> +	 * For Targets which don't have SSBS support, they
>> +	 * can return from here.
>> +	 */
>> +	if (!IS_ENABLED(CONFIG_ARM64_SSBD))
>> +		return;
> 
> Does this actually make a measurable difference?
> 
> Will
> 

Hi Will,

While doing code review between older kernel and latest kernel for 
context switch case, there i have found this and thought it is good to 
have for non-ssbs system to return early(as this might improve).

Please let me know if you want to run some tests.

Regards
Gaurav

Re: [PATCH] arm64: Skip apply SSBS call for non SSBS system

[Gaurav Kohli]

On 8/12/2020 7:00 PM, Will Deacon wrote:
> On Tue, Aug 04, 2020 at 07:44:42PM +0530, Gaurav Kohli wrote:
>> In a system where no cpu's implement SSBS, for
>> them no need to set pstate. This might help to save
>> few cpu cycles during context switch.
>>
>> Signed-off-by: Gaurav Kohli <gkohli@codeaurora.org>
>>
>> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
>> index 6089638..79f80f1 100644
>> --- a/arch/arm64/kernel/process.c
>> +++ b/arch/arm64/kernel/process.c
>> @@ -477,6 +477,13 @@ static void ssbs_thread_switch(struct task_struct *next)
>>   	struct pt_regs *regs = task_pt_regs(next);
>>   
>>   	/*
>> +	 * For Targets which don't have SSBS support, they
>> +	 * can return from here.
>> +	 */
>> +	if (!IS_ENABLED(CONFIG_ARM64_SSBD))
>> +		return;
> 
> Does this actually make a measurable difference?
> 
> Will
> 

Hi Will,

While doing code review between older kernel and latest kernel for 
context switch case, there i have found this and thought it is good to 
have for non-ssbs system to return early(as this might improve).

Please let me know if you want to run some tests.

Regards
Gaurav

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel