regmap: debugfs: Allow writes to cache state settings

PATCH Superseded Richard Fitzgerald 2 participants 6 messages Jun 22, 2015 – Jun 23, 2015

Subsystems (2)

Driver core Register map abstraction

lore

Share & tools

lore

Richard Fitzgerald

author

2015-06-22 15:28 UTC #0 lore raw

cover regmap: debugfs: Allow writes to cache state settings

The was based on broonie/regmap.git for-next

Richard Fitzgerald (1):
regmap: debugfs: Allow writes to cache state settings

 drivers/base/regmap/regmap-debugfs.c |  135 +++++++++++++++++++++++++++++++++-
 1 files changed, 131 insertions(+), 4 deletions(-)

Signature · 5 lines

1.7.2.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Richard Fitzgerald

author

2015-06-22 15:28 UTC #1 lore raw

PATCH 1/1 regmap: debugfs: Allow writes to cache state settings

Allow the user to write the cache_only and cache_bypass settings. This can be useful for debugging.

Since this can lead to the hardware getting out-of-sync with the cache, at least for the period that the cache state is forced, the kernel is tainted and the action is recorded in the kernel log.

When disabling cache_only through debugfs a cache sync will be performed.

Signed-off-by Richard Fitzgerald <[email protected]>

---
 drivers/base/regmap/regmap-debugfs.c |  135 +++++++++++++++++++++++++++++++++-
 1 files changed, 131 insertions(+), 4 deletions(-)

drivers/base/regmap/regmap-debugfs.c +131 -4

index 5799a0b..4ad6a1a 100644
@@ -469,6 +469,133 @@ static const struct file_operations regmap_access_fops = {
 	.llseek = default_llseek,
 };
 
+static ssize_t regmap_bool_read_file(struct file *file,
+				     char __user *user_buf,
+				     size_t count, loff_t *ppos,
+				     bool value)
+{
+	char buf[2];
+
+	buf[0] = value ? 'Y' : 'N';
+	buf[1] = '\n';
+	return simple_read_from_buffer(user_buf, count, ppos, buf, sizeof(buf));
+}
+
+static ssize_t regmap_bool_write_file(struct file *file,
+				      const char __user *user_buf,
+				      size_t count, loff_t *ppos,
+				      bool *value)
+{
+	char buf[32];
+	size_t buf_size;
+
+	buf_size = min(count, (sizeof(buf)-1));
+	if (copy_from_user(buf, user_buf, buf_size))
+		return -EFAULT;
+
+	buf[buf_size] = '\0';
+	if (strtobool(buf, value) != 0)
+		return -EINVAL;
+
+	return count;
+}
+
+static ssize_t regmap_cache_only_read_file(struct file *file,
+					   char __user *user_buf,
+					   size_t count, loff_t *ppos)
+{
+	struct regmap *map = file->private_data;
+
+	return regmap_bool_read_file(file, user_buf, count, ppos,
+				     map->cache_only);
+}
+
+static ssize_t regmap_cache_only_write_file(struct file *file,
+					    const char __user *user_buf,
+					    size_t count, loff_t *ppos)
+{
+	struct regmap *map = file->private_data;
+	ssize_t result;
+	bool enable, require_sync = false;
+	int err;
+
+	result = regmap_bool_write_file(file, user_buf, count, ppos, &enable);
+	if (result < 0)
+		return result;
+
+	map->lock(map->lock_arg);
+
+	if (enable && !map->cache_only) {
+		dev_warn(map->dev, "debugfs cache_only=Y forced\n");
+		add_taint(TAINT_USER, LOCKDEP_STILL_OK);
+		map->cache_only = true;
+	} else if (!enable && map->cache_only) {
+		dev_warn(map->dev, "debugfs cache_only=N forced: syncing cache\n");
+		map->cache_only = false;
+		require_sync = true;
+	}
+
+	map->unlock(map->lock_arg);
+
+	if (require_sync) {
+		err = regcache_sync(map);
+		if (err)
+			dev_err(map->dev, "Failed to sync cache %d\n", err);
+	}
+
+	return result;
+}
+
+static const struct file_operations regmap_cache_only_fops = {
+	.open = simple_open,
+	.read = regmap_cache_only_read_file,
+	.write = regmap_cache_only_write_file,
+};
+
+static ssize_t regmap_cache_bypass_read_file(struct file *file,
+					     char __user *user_buf,
+					     size_t count, loff_t *ppos)
+{
+	struct regmap *map = file->private_data;
+
+	return regmap_bool_read_file(file, user_buf, count, ppos,
+				     map->cache_bypass);
+}
+
+static ssize_t regmap_cache_bypass_write_file(struct file *file,
+					      const char __user *user_buf,
+					      size_t count, loff_t *ppos)
+{
+	struct regmap *map = file->private_data;
+	ssize_t result;
+	bool enable;
+
+	result = regmap_bool_write_file(file, user_buf, count, ppos, &enable);
+	if (result < 0)
+		return result;
+
+	map->lock(map->lock_arg);
+
+	if (enable && !map->cache_bypass) {
+		dev_warn(map->dev, "debugfs cache_bypass=Y forced\n");
+		add_taint(TAINT_USER, LOCKDEP_STILL_OK);
+		map->cache_bypass = true;
+	} else if (!enable && map->cache_bypass) {
+		dev_warn(map->dev, "debugfs cache_bypass=N forced\n");
+		map->cache_bypass = false;
+	}
+
+	map->unlock(map->lock_arg);
+
+	return result;
+}
+
+static const struct file_operations regmap_cache_bypass_fops = {
+	.open = simple_open,
+	.read = regmap_cache_bypass_read_file,
+	.write = regmap_cache_bypass_write_file,
+};
+
 void regmap_debugfs_init(struct regmap *map, const char *name)
 {
 	struct rb_node *next;

@@ -530,12 +657,12 @@ void regmap_debugfs_init(struct regmap *map, const char *name)
 	}
 
 	if (map->cache_type) {
-		debugfs_create_bool("cache_only", 0400, map->debugfs,
-				    &map->cache_only);
+		debugfs_create_file("cache_only", 0600, map->debugfs,
+				    map, &regmap_cache_only_fops);
 		debugfs_create_bool("cache_dirty", 0400, map->debugfs,
 				    &map->cache_dirty);
-		debugfs_create_bool("cache_bypass", 0400, map->debugfs,
-				    &map->cache_bypass);
+		debugfs_create_file("cache_bypass", 0600, map->debugfs,
+				    map, &regmap_cache_bypass_fops);
 	}
 
 	next = rb_first(&map->range_tree);

Signature · 5 lines

1.7.2.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Mark Brown

maintainer

2015-06-22 15:49 UTC #2 lore raw

On Mon, Jun 22, 2015 at 04:30:34PM +0100, Richard Fitzgerald wrote:

This looks basically good but:

Quoted 7 lines

drivers/base/regmap/regmap-debugfs.c +7 -0

+static ssize_t regmap_bool_read_file(struct file *file,
+				     char __user *user_buf,
+				     size_t count, loff_t *ppos,
+				     bool value)
+{
+	char buf[2];
+

drivers/base/regmap/regmap-debugfs.c static const struct file_operations regmap_access_fops = { · line 488

Quoted 10 lines

drivers/base/regmap/regmap-debugfs.c +9 -0

+	buf[0] = value ? 'Y' : 'N';
+	buf[1] = '\n';
+	return simple_read_from_buffer(user_buf, count, ppos, buf, sizeof(buf));
+}
 
+static ssize_t regmap_bool_write_file(struct file *file,
+				      const char __user *user_buf,
+				      size_t count, loff_t *ppos,
+				      bool *value)
+{

Can we not export read/write_file_bool() instead? For the read this seems to be an almost cut'n'paste with slight differences that look like taste changes. For the write path we can just stash the current value in the calling functions instead of cut'n'pasting the code.

Richard Fitzgerald

author

2015-06-22 16:10 UTC #3 lore raw

Quoted 16 lines · Mark Brown

On Mon, Jun 22, 2015 at 04:30:34PM +0100, Richard Fitzgerald wrote:

This looks basically good but:

Quoted 11 lines

drivers/base/regmap/regmap-debugfs.c +11 -0

+static ssize_t regmap_bool_read_file(struct file *file,
+				     char __user *user_buf,
+				     size_t count, loff_t *ppos,
+				     bool value)
+{
+	char buf[2];
+
+	buf[0] = value ? 'Y' : 'N';
+	buf[1] = '\n';
+	return simple_read_from_buffer(user_buf, count, ppos, buf, sizeof(buf));
+}

Quoted 10 lines

drivers/base/regmap/regmap-debugfs.c static const struct file_operations regmap_access_fops = { · line 488
Quoted 5 lines
drivers/base/regmap/regmap-debugfs.c +5 -0
+static ssize_t regmap_bool_write_file(struct file *file,
+				      const char __user *user_buf,
+				      size_t count, loff_t *ppos,
+				      bool *value)
+{
Can we not export read/write_file_bool() instead? For the read this seems to be an almost cut'n'paste with slight differences that look like taste changes. For the write path we can just stash the current value in the calling functions instead of cut'n'pasting the code.

They assume that file->private_data points at the actual bool, and it doesn't here, which means temporarily patching it around the call. --
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in Please read the FAQ at http://www.tux.org/lkml/

Mark Brown

maintainer

2015-06-23 10:27 UTC #4 lore raw

Quoted 9 lines · Richard Fitzgerald

Quoted 4 lines · Mark Brown
Can we not export read/write_file_bool() instead? For the read this seems to be an almost cut'n'paste with slight differences that look like taste changes. For the write path we can just stash the current value in the calling functions instead of cut'n'pasting the code.
They assume that file->private_data points at the actual bool, and it doesn't here, which means temporarily patching it around the call.

Or you could use container_of() in your wrappers for the write path.

Mark Brown

maintainer

2015-06-22 15:37 UTC #5 lore raw

In reply to Richard Fitzgerald

Quoted 7 lines · Richard Fitzgerald

The was based on broonie/regmap.git for-next
Richard Fitzgerald (1):
regmap: debugfs: Allow writes to cache state settings
 drivers/base/regmap/regmap-debugfs.c |  135 +++++++++++++++++++++++++++++++++-
 1 files changed, 131 insertions(+), 4 deletions(-)

Please don't send cover letters for single patches, if there is any content in them it should be in the changelog or after the --- in the mail if it's administrative.