linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RESEND] regmap: introduce tables for readable/writeable/volatile/precious checks
@ 2012-11-15 12:46 ciminaghi
  2012-11-16  1:19 ` Mark Brown
  0 siblings, 1 reply; 3+ messages in thread
From: ciminaghi @ 2012-11-15 12:46 UTC (permalink / raw)
  To: broonie, gregkh; +Cc: linux-kernel, Davide Ciminaghi

From: Davide Ciminaghi <ciminaghi@gnudd.com>

Many of the regmap enabled drivers implementing one or more of the
readable, writeable, volatile and precious methods use the same code
pattern:

	return ((reg >= X && reg <= Y) || (reg >= W && reg <= Z) || ...)

Switch to a data driven approach, using tables to describe
readable/writeable/volatile and precious registers ranges instead.
The table based check can still be overridden by passing the usual function
pointers via struct regmap_config.

Signed-off-by: Davide Ciminaghi <ciminaghi@gnudd.com>
---

I haven't seen any reply up to now, so I'm just resending the original patch
(see https://lkml.org/lkml/2012/10/25/224).
I just applied it to today's next and did a quick re-test.

 drivers/base/regmap/internal.h |    4 +++
 drivers/base/regmap/regmap.c   |   50 +++++++++++++++++++++++++++
 include/linux/regmap.h         |   73 ++++++++++++++++++++++++++++++++++------
 3 files changed, 116 insertions(+), 11 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 2cd01b5..c6e64e0 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -55,6 +55,10 @@ struct regmap {
 	bool (*readable_reg)(struct device *dev, unsigned int reg);
 	bool (*volatile_reg)(struct device *dev, unsigned int reg);
 	bool (*precious_reg)(struct device *dev, unsigned int reg);
+	const struct regmap_range_table *wr_table;
+	const struct regmap_range_table *rd_table;
+	const struct regmap_range_table *volatile_table;
+	const struct regmap_range_table *precious_table;
 
 	u8 read_flag_mask;
 	u8 write_flag_mask;
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 273bf30..054b3c4 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -34,6 +34,40 @@ static int _regmap_update_bits(struct regmap *map, unsigned int reg,
 			       unsigned int mask, unsigned int val,
 			       bool *change);
 
+static inline bool _reg_in_range(unsigned int reg,
+				 const struct regmap_range *range)
+{
+	return reg >= range->range_min && reg <= range->range_max;
+}
+
+static inline bool _reg_in_ranges(unsigned int reg,
+				  const struct regmap_range *ranges,
+				  unsigned int nranges)
+{
+	const struct regmap_range *r;
+	int i;
+
+	for (i = 0, r = ranges; i < nranges; i++, r++)
+		if (_reg_in_range(reg, r))
+			return true;
+	return false;
+}
+
+static bool _regmap_check_range_table(struct regmap *map,
+				      unsigned int reg,
+				      const struct regmap_range_table *table)
+{
+	/* Check "no ranges" first */
+	if (_reg_in_ranges(reg, table->no_ranges, table->n_no_ranges))
+		return false;
+
+	/* In case zero "yes ranges" are supplied, any reg is OK */
+	if (!table->n_yes_ranges)
+		return true;
+
+	return _reg_in_ranges(reg, table->yes_ranges, table->n_yes_ranges);
+}
+
 bool regmap_writeable(struct regmap *map, unsigned int reg)
 {
 	if (map->max_register && reg > map->max_register)
@@ -42,6 +76,9 @@ bool regmap_writeable(struct regmap *map, unsigned int reg)
 	if (map->writeable_reg)
 		return map->writeable_reg(map->dev, reg);
 
+	if (map->wr_table)
+		return _regmap_check_range_table(map, reg, map->wr_table);
+
 	return true;
 }
 
@@ -56,6 +93,9 @@ bool regmap_readable(struct regmap *map, unsigned int reg)
 	if (map->readable_reg)
 		return map->readable_reg(map->dev, reg);
 
+	if (map->rd_table)
+		return _regmap_check_range_table(map, reg, map->rd_table);
+
 	return true;
 }
 
@@ -67,6 +107,9 @@ bool regmap_volatile(struct regmap *map, unsigned int reg)
 	if (map->volatile_reg)
 		return map->volatile_reg(map->dev, reg);
 
+	if (map->volatile_table)
+		return _regmap_check_range_table(map, reg, map->volatile_table);
+
 	return true;
 }
 
@@ -78,6 +121,9 @@ bool regmap_precious(struct regmap *map, unsigned int reg)
 	if (map->precious_reg)
 		return map->precious_reg(map->dev, reg);
 
+	if (map->precious_table)
+		return _regmap_check_range_table(map, reg, map->precious_table);
+
 	return false;
 }
 
@@ -370,6 +416,10 @@ struct regmap *regmap_init(struct device *dev,
 	map->bus = bus;
 	map->bus_context = bus_context;
 	map->max_register = config->max_register;
+	map->wr_table = config->wr_table;
+	map->rd_table = config->rd_table;
+	map->volatile_table = config->volatile_table;
+	map->precious_table = config->precious_table;
 	map->writeable_reg = config->writeable_reg;
 	map->readable_reg = config->readable_reg;
 	map->volatile_reg = config->volatile_reg;
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index fedf7ba..e4e0ef6 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -54,6 +54,36 @@ enum regmap_endian {
 	REGMAP_ENDIAN_NATIVE,
 };
 
+/**
+ * A register range, used for access related checks
+ * (readable/writeable/volatile/precious checks)
+ *
+ * @range_min: address of first register
+ * @range_max: address of last register
+ */
+struct regmap_range {
+	unsigned int range_min;
+	unsigned int range_max;
+};
+
+/*
+ * A table of ranges including some yes ranges and some no ranges.
+ * If a register belongs to a no_range, the corresponding check function
+ * will return false. If a register belongs to a yes range, the corresponding
+ * check function will return true. "no_ranges" are searched first.
+ *
+ * @yes_ranges : pointer to an array of regmap ranges used as "yes ranges"
+ * @n_yes_ranges: size of the above array
+ * @no_ranges: pointer to an array of regmap ranges used as "no ranges"
+ * @n_no_ranges: size of the above array
+ */
+struct regmap_range_table {
+	const struct regmap_range *yes_ranges;
+	unsigned int n_yes_ranges;
+	const struct regmap_range *no_ranges;
+	unsigned int n_no_ranges;
+};
+
 typedef void (*regmap_lock)(void *);
 typedef void (*regmap_unlock)(void *);
 
@@ -71,22 +101,39 @@ typedef void (*regmap_unlock)(void *);
  * @val_bits: Number of bits in a register value, mandatory.
  *
  * @writeable_reg: Optional callback returning true if the register
- *                 can be written to.
+ *		   can be written to. If this field is NULL but wr_table
+ *		   (see below) is not, the check is performed on such table
+ *                 (a register is writeable if it belongs to one of the ranges
+ *                  specified by wr_table).
  * @readable_reg: Optional callback returning true if the register
- *                can be read from.
+ *		  can be read from. If this field is NULL but rd_table
+ *		   (see below) is not, the check is performed on such table
+ *                 (a register is readable if it belongs to one of the ranges
+ *                  specified by rd_table).
  * @volatile_reg: Optional callback returning true if the register
- *                value can't be cached.
+ *		  value can't be cached. If this field is NULL but
+ *		  volatile_table (see below) is not, the check is performed on
+ *                such table (a register is volatile if it belongs to one of
+ *                the ranges specified by volatile_table).
  * @precious_reg: Optional callback returning true if the rgister
- *                should not be read outside of a call from the driver
- *                (eg, a clear on read interrupt status register).
- * @lock:         Optional lock callback (overrides regmap's default lock
- *                function, based on spinlock or mutex).
- * @unlock:       As above for unlocking.
- * @lock_arg:     this field is passed as the only argument of lock/unlock
- *                functions (ignored in case regular lock/unlock functions
- *                are not overridden).
+ *		  should not be read outside of a call from the driver
+ *		  (eg, a clear on read interrupt status register). If this
+ *                field is NULL but precious_table (see below) is not, the
+ *                check is performed on such table (a register is precious if
+ *                it belongs to one of the ranges specified by precious_table).
+ * @lock:	  Optional lock callback (overrides regmap's default lock
+ *		  function, based on spinlock or mutex).
+ * @unlock:	  As above for unlocking.
+ * @lock_arg:	  this field is passed as the only argument of lock/unlock
+ *		  functions (ignored in case regular lock/unlock functions
+ *		  are not overridden).
  *
  * @max_register: Optional, specifies the maximum valid register index.
+ * @wr_table:     Optional, points to a struct regmap_range_table specifying
+ *                valid ranges for write access.
+ * @rd_table:     As above, for read access.
+ * @volatile_table: As above, for volatile registers.
+ * @precious_table: As above, for precious registers.
  * @reg_defaults: Power on reset values for registers (for use with
  *                register cache support).
  * @num_reg_defaults: Number of elements in reg_defaults.
@@ -131,6 +178,10 @@ struct regmap_config {
 	void *lock_arg;
 
 	unsigned int max_register;
+	const struct regmap_range_table *wr_table;
+	const struct regmap_range_table *rd_table;
+	const struct regmap_range_table *volatile_table;
+	const struct regmap_range_table *precious_table;
 	const struct reg_default *reg_defaults;
 	unsigned int num_reg_defaults;
 	enum regcache_type cache_type;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH RESEND] regmap: introduce tables for readable/writeable/volatile/precious checks
  2012-11-15 12:46 [PATCH RESEND] regmap: introduce tables for readable/writeable/volatile/precious checks ciminaghi
@ 2012-11-16  1:19 ` Mark Brown
  2012-11-16 23:53   ` Davide Ciminaghi
  0 siblings, 1 reply; 3+ messages in thread
From: Mark Brown @ 2012-11-16  1:19 UTC (permalink / raw)
  To: ciminaghi; +Cc: gregkh, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1037 bytes --]

On Thu, Nov 15, 2012 at 01:46:52PM +0100, ciminaghi@gnudd.com wrote:

> I haven't seen any reply up to now, so I'm just resending the original patch
> (see https://lkml.org/lkml/2012/10/25/224).
> I just applied it to today's next and did a quick re-test.

The fact that you don't send a real name in your patch submission mails
might be an issue here - I suspect you've fallen foul of spam filtering,
either automatic or human, there's no sign of this in my pending queue.

> +static inline bool _reg_in_ranges(unsigned int reg,
> +				  const struct regmap_range *ranges,
> +				  unsigned int nranges)
> +{
> +	const struct regmap_range *r;
> +	int i;
> +
> +	for (i = 0, r = ranges; i < nranges; i++, r++)
> +		if (_reg_in_range(reg, r))
> +			return true;
> +	return false;
> +}

It would be better to make this an externally visible function (perhaps
just move it into the header with a regmap on the front of the name) so
that drivers can use this as part of the implementation of more complex
checks.  Otherwise this looks fine.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH RESEND] regmap: introduce tables for readable/writeable/volatile/precious checks
  2012-11-16  1:19 ` Mark Brown
@ 2012-11-16 23:53   ` Davide Ciminaghi
  0 siblings, 0 replies; 3+ messages in thread
From: Davide Ciminaghi @ 2012-11-16 23:53 UTC (permalink / raw)
  To: Mark Brown; +Cc: gregkh, linux-kernel

On Fri, Nov 16, 2012 at 10:19:07AM +0900, Mark Brown wrote:
> On Thu, Nov 15, 2012 at 01:46:52PM +0100, ciminaghi@gnudd.com wrote:
> 
> > I haven't seen any reply up to now, so I'm just resending the original patch
> > (see https://lkml.org/lkml/2012/10/25/224).
> > I just applied it to today's next and did a quick re-test.
> 
> The fact that you don't send a real name in your patch submission mails
> might be an issue here - I suspect you've fallen foul of spam filtering,
> either automatic or human, there's no sign of this in my pending queue.
>
mmm maybe I did something wrong with git send-email. Will check, thanks for
pointing this out.
 
> > +static inline bool _reg_in_ranges(unsigned int reg,
> > +				  const struct regmap_range *ranges,
> > +				  unsigned int nranges)
> > +{
> > +	const struct regmap_range *r;
> > +	int i;
> > +
> > +	for (i = 0, r = ranges; i < nranges; i++, r++)
> > +		if (_reg_in_range(reg, r))
> > +			return true;
> > +	return false;
> > +}
> 
> It would be better to make this an externally visible function (perhaps
> just move it into the header with a regmap on the front of the name) so
> that drivers can use this as part of the implementation of more complex
> checks.  Otherwise this looks fine.

ok, I'll fix the patch and resend it next week.

Thanks and regards
Davide


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-11-16 23:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-15 12:46 [PATCH RESEND] regmap: introduce tables for readable/writeable/volatile/precious checks ciminaghi
2012-11-16  1:19 ` Mark Brown
2012-11-16 23:53   ` Davide Ciminaghi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).