2012-11-15 12:47:25

by Davide Ciminaghi

[permalink] [raw]
Subject: [PATCH RESEND] regmap: introduce tables for readable/writeable/volatile/precious checks

From: Davide Ciminaghi <[email protected]>

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]>
---

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


2012-11-16 01:20:03

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH RESEND] regmap: introduce tables for readable/writeable/volatile/precious checks

On Thu, Nov 15, 2012 at 01:46:52PM +0100, [email protected] 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.


Attachments:
(No filename) (1.01 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-11-16 23:53:16

by Davide Ciminaghi

[permalink] [raw]
Subject: Re: [PATCH RESEND] regmap: introduce tables for readable/writeable/volatile/precious checks

On Fri, Nov 16, 2012 at 10:19:07AM +0900, Mark Brown wrote:
> On Thu, Nov 15, 2012 at 01:46:52PM +0100, [email protected] 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