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 <[email protected]>
---
Mark, following your comments I just did a couple of changes:
_reg_in_ranges made external and renamed (regmap_reg_in_rages).
_reg_in_range moved to regmap.h and renamed (regmap_reg_in_range), maybe
it could be useful.
drivers/base/regmap/internal.h | 4 ++
drivers/base/regmap/regmap.c | 45 ++++++++++++++++++++++
include/linux/regmap.h | 82 ++++++++++++++++++++++++++++++++++------
3 files changed, 120 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..056aa78 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -34,6 +34,35 @@ static int _regmap_update_bits(struct regmap *map, unsigned int reg,
unsigned int mask, unsigned int val,
bool *change);
+bool regmap_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 (regmap_reg_in_range(reg, r))
+ return true;
+ return false;
+}
+EXPORT_SYMBOL_GPL(regmap_reg_in_ranges);
+
+static bool _regmap_check_range_table(struct regmap *map,
+ unsigned int reg,
+ const struct regmap_range_table *table)
+{
+ /* Check "no ranges" first */
+ if (regmap_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 regmap_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 +71,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 +88,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 +102,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 +116,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 +411,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..f467cba 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;
@@ -281,6 +332,15 @@ void regcache_mark_dirty(struct regmap *map);
int regmap_register_patch(struct regmap *map, const struct reg_default *regs,
int num_regs);
+static inline bool regmap_reg_in_range(unsigned int reg,
+ const struct regmap_range *range)
+{
+ return reg >= range->range_min && reg <= range->range_max;
+}
+
+bool regmap_reg_in_ranges(unsigned int reg, const struct regmap_range *ranges,
+ unsigned int nranges);
+
/**
* Description of an IRQ for the generic regmap irq_chip.
*
--
1.7.10.4
On Mon, Nov 19, 2012 at 02:31:53PM +0100, Davide Ciminaghi wrote:
> 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.
Sorry, the code looks good but I just realised we already have a feature
called ranges - the paged window support also calls the regions it uses
ranges. Can I suggest access_table or similar instead? Sorry about
that - I should've realised.
On Tue, Nov 20, 2012 at 03:05:11PM +0900, Mark Brown wrote:
> On Mon, Nov 19, 2012 at 02:31:53PM +0100, Davide Ciminaghi wrote:
> > 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.
>
> Sorry, the code looks good but I just realised we already have a feature
> called ranges - the paged window support also calls the regions it uses
> ranges. Can I suggest access_table or similar instead? Sorry about
> that - I should've realised.
ok no problem, I'll change the name and resend.
Thanks and regards
Davide