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

Groups > linux.kernel > #1245975 > unrolled thread

[PATCH] regmap: Add a config option for making regmap debugfs writable

Back to article view | Back to linux.kernel

Contents

  [PATCH] regmap: Add a config option for making regmap debugfs writable Anatol Pomozov <anatol.pomozov@gmail.com> - 2015-10-13 19:50 +0200
    Re: [PATCH] regmap: Add a config option for making regmap debugfs  writable Mark Brown <broonie@kernel.org> - 2015-10-13 20:00 +0200
      Re: [PATCH] regmap: Add a config option for making regmap debugfs writable Anatol Pomozov <anatol.pomozov@gmail.com> - 2015-10-13 20:40 +0200
        Re: [PATCH] regmap: Add a config option for making regmap debugfs  writable Mark Brown <broonie@kernel.org> - 2015-10-13 22:10 +0200

#1245975 — [PATCH] regmap: Add a config option for making regmap debugfs writable

Instead of modifiying source code directly one should use config files.
It is the standard way to set compile-time options.

Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>
---
 drivers/base/regmap/Kconfig          | 6 ++++++
 drivers/base/regmap/regmap-debugfs.c | 5 ++---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
index db9d00c3..579c8b5 100644
--- a/drivers/base/regmap/Kconfig
+++ b/drivers/base/regmap/Kconfig
@@ -9,6 +9,12 @@ config REGMAP
 	select IRQ_DOMAIN if REGMAP_IRQ
 	bool
 
+config REGMAP_ALLOW_WRITE_DEBUGFS
+	bool "Make regmap debugfs writable"
+	default n
+	select REGMAP
+	select DEBUG_FS
+
 config REGMAP_AC97
 	tristate
 
diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
index 3f0a7e2..3ea2e27 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -259,8 +259,7 @@ static ssize_t regmap_map_read_file(struct file *file, char __user *user_buf,
 				   count, ppos);
 }
 
-#undef REGMAP_ALLOW_WRITE_DEBUGFS
-#ifdef REGMAP_ALLOW_WRITE_DEBUGFS
+#ifdef CONFIG_REGMAP_ALLOW_WRITE_DEBUGFS
 /*
  * This can be dangerous especially when we have clients such as
  * PMICs, therefore don't provide any real compile time configuration option
@@ -595,7 +594,7 @@ void regmap_debugfs_init(struct regmap *map, const char *name)
 	if (map->max_register || regmap_readable(map, 0)) {
 		umode_t registers_mode;
 
-#if defined(REGMAP_ALLOW_WRITE_DEBUGFS)
+#ifdef CONFIG_REGMAP_ALLOW_WRITE_DEBUGFS
 		registers_mode = 0600;
 #else
 		registers_mode = 0400;
-- 
2.6.0.rc2.230.g3dd15c0

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

[toc] | [next] | [standalone]

#1245998 — Re: [PATCH] regmap: Add a config option for making regmap debugfs writable

[Multipart message — attachments visible in raw view] — view raw

On Tue, Oct 13, 2015 at 10:46:55AM -0700, Anatol Pomozov wrote:
> Instead of modifiying source code directly one should use config files.
> It is the standard way to set compile-time options.

This is deliberately not a Kconfig option because it is a terrible idea
to do this in production and making it either selectable or the default
is an invitation to abuse.  We want to place a barrier here so that
users know that this is something that they have taken a decision to
enable, not something that is in any way supported (this is also why we
taint the kernel when people do write).

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

#1246041

Hi

On Tue, Oct 13, 2015 at 10:55 AM, Mark Brown <broonie@kernel.org> wrote:
> This is deliberately not a Kconfig option because it is a terrible idea
> to do this in production and making it either selectable or the default
> is an invitation to abuse.

What kind of abuse are you talking about?

Having an easy way of modifying chip registers is extremely useful
during bringup / driver development. And during device development
phase I regularly have situations when I need to change a register to
see if it fixes an issue. Sometimes I need to test it remotely when
users located at another end of the Earth.

Current kernel source suggests I need to modify regmap-debugfs.c
directly. But my kernel tree is shared by multiply products and some
of the products in production already. I do not want to enable
writable remap for production products. I would like to have a
per-product compile-time configuration and .config serves exactly this
purpose.


> We want to place a barrier here so that
> users know that this is something that they have taken a decision to
> enable, not something that is in any way supported (this is also why we
> taint the kernel when people do write).

Honestly I am not convinced. Why to put obstacles on a feature that is
very useful during development?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1246104 — Re: [PATCH] regmap: Add a config option for making regmap debugfs writable

[Multipart message — attachments visible in raw view] — view raw

On Tue, Oct 13, 2015 at 11:33:13AM -0700, Anatol Pomozov wrote:
> On Tue, Oct 13, 2015 at 10:55 AM, Mark Brown <broonie@kernel.org> wrote:

> > This is deliberately not a Kconfig option because it is a terrible idea
> > to do this in production and making it either selectable or the default
> > is an invitation to abuse.

> What kind of abuse are you talking about?

Using it as a standard interface to control systems in production rather
than having appropriate support in the relevant driver.

> Having an easy way of modifying chip registers is extremely useful
> during bringup / driver development. And during device development
> phase I regularly have situations when I need to change a register to
> see if it fixes an issue. Sometimes I need to test it remotely when
> users located at another end of the Earth.

This is exactly the sort of use case this feature is intended for, and
is the sort of situation where a custom kernel is not going to be any
kind of practical problem.

> Current kernel source suggests I need to modify regmap-debugfs.c
> directly. But my kernel tree is shared by multiply products and some
> of the products in production already. I do not want to enable
> writable remap for production products. I would like to have a
> per-product compile-time configuration and .config serves exactly this
> purpose.

Feel free to make that modification in your local tree if you want it,
I'm not going to take it for upstream.

> > We want to place a barrier here so that
> > users know that this is something that they have taken a decision to
> > enable, not something that is in any way supported (this is also why we
> > taint the kernel when people do write).

> Honestly I am not convinced. Why to put obstacles on a feature that is
> very useful during development?

We don't want people complaining when someone misprograms their PMIC or
battery charger in a production system because a debug feature got left
on by mistake (both components that frequently use regmap and both
components that have the capacity to physically damage the system), or
have someone decide that the way to tune their system is to turn on this
option and bang on the hardware from userspace bypassing the driver.
It's really handy for debug but it's terrible for system robustness and
stability.

The whole point is that this is only intended to be used during
development while modifying the kernel, if you're able to do that it's
not a meningful obstacle.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web