[alsa-devel] [PATCH 0/6][RFC] cleanup regmap write functions
Hi Mark
Current regmap has many similar functions regmap_update_bits() regmap_update_bits_async() regmap_update_bits_check() regmap_update_bits_check_async() But difference is very few. And I would like to have _force_ feature on it. So, these patches add new regmap_raw_update_bits() which has _check, _async, _force option. Above functions are now defined as macro. 4), 5) adds _force_ feature. We can easily add _check, _async.
I used [RFC], because regmap has many effect.
BTW, I noticed #if - #else - #endif on ${LINUX}/include/linux/regmap.h are strange. Many functions/struct/macro are defined under #ifdef CONFIG_REGMAP, but few are defined under #else. It can be trouble ? Do we really need this #if ?
Kuninori Morimoto (7): 1) regmap: add regmap_raw_update_bits() and merge all regmap_update_bits_xxx() 2) regmap: regmap_field_xxx() uses regmap_raw_update_bits() 3) regmap: regmap_fields_xxx() uses regmap_raw_update_bits() 4) regmap: add regmap_field_force_xxx() functions 5) regmap: add regmap_fields_force_xxx() functions 6) regmpa: remove regmap_write_bits() 7) ASoC: rsnd: rsnd_write() / rsnd_bset() uses regmap _force_ function
drivers/base/regmap/regmap.c | 191 +++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------ include/linux/regmap.h | 102 ++++++++++++++++++++++++++++++---------------------------------------- sound/soc/sh/rcar/gen.c | 21 ++------------- sound/soc/sh/rcar/rsnd.h | 2 -- 4 files changed, 88 insertions(+), 228 deletions(-)
Best regards --- Kuninori Morimoto
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Current regmap has many similar update functions, but the difference is very few. This patch adds new regmap_raw_update_bits() and merge all update functions into it by macro. regmap_update_bits() regmap_update_bits_async() regmap_update_bits_check() regmap_update_bits_check_async()
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- drivers/base/regmap/regmap.c | 113 ++++++------------------------------------- include/linux/regmap.h | 54 ++++++--------------- 2 files changed, 31 insertions(+), 136 deletions(-)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c index ee54e84..c91e67b 100644 --- a/drivers/base/regmap/regmap.c +++ b/drivers/base/regmap/regmap.c @@ -2648,76 +2648,34 @@ static int _regmap_update_bits(struct regmap *map, unsigned int reg, }
/** - * regmap_update_bits: Perform a read/modify/write cycle on the register map - * - * @map: Register map to update - * @reg: Register to update - * @mask: Bitmask to change - * @val: New value for bitmask - * - * Returns zero for success, a negative number on error. - */ -int regmap_update_bits(struct regmap *map, unsigned int reg, - unsigned int mask, unsigned int val) -{ - int ret; - - map->lock(map->lock_arg); - ret = _regmap_update_bits(map, reg, mask, val, NULL, false); - map->unlock(map->lock_arg); - - return ret; -} -EXPORT_SYMBOL_GPL(regmap_update_bits); - -/** - * regmap_write_bits: Perform a read/modify/write cycle on the register map - * - * @map: Register map to update - * @reg: Register to update - * @mask: Bitmask to change - * @val: New value for bitmask - * - * Returns zero for success, a negative number on error. - */ -int regmap_write_bits(struct regmap *map, unsigned int reg, - unsigned int mask, unsigned int val) -{ - int ret; - - map->lock(map->lock_arg); - ret = _regmap_update_bits(map, reg, mask, val, NULL, true); - map->unlock(map->lock_arg); - - return ret; -} -EXPORT_SYMBOL_GPL(regmap_write_bits); - -/** - * regmap_update_bits_async: Perform a read/modify/write cycle on the register - * map asynchronously + * regmap_raw_update_bits: Perform a read/modify/write cycle on the register map * * @map: Register map to update * @reg: Register to update * @mask: Bitmask to change * @val: New value for bitmask + * @change: Boolean indicating if a write was done + * @async: Boolean indicating asynchronously + * @force: Boolean indicating use force update * + * if async was true, * With most buses the read must be done synchronously so this is most * useful for devices with a cache which do not need to interact with * the hardware to determine the current register value. * * Returns zero for success, a negative number on error. */ -int regmap_update_bits_async(struct regmap *map, unsigned int reg, - unsigned int mask, unsigned int val) +int regmap_raw_update_bits(struct regmap *map, unsigned int reg, + unsigned int mask, unsigned int val, + bool *change, bool async, bool force) { int ret;
map->lock(map->lock_arg);
- map->async = true; + map->async = async ? true : false;
- ret = _regmap_update_bits(map, reg, mask, val, NULL, false); + ret = _regmap_update_bits(map, reg, mask, val, change, force);
map->async = false;
@@ -2725,69 +2683,30 @@ int regmap_update_bits_async(struct regmap *map, unsigned int reg,
return ret; } -EXPORT_SYMBOL_GPL(regmap_update_bits_async); - -/** - * regmap_update_bits_check: Perform a read/modify/write cycle on the - * register map and report if updated - * - * @map: Register map to update - * @reg: Register to update - * @mask: Bitmask to change - * @val: New value for bitmask - * @change: Boolean indicating if a write was done - * - * Returns zero for success, a negative number on error. - */ -int regmap_update_bits_check(struct regmap *map, unsigned int reg, - unsigned int mask, unsigned int val, - bool *change) -{ - int ret; - - map->lock(map->lock_arg); - ret = _regmap_update_bits(map, reg, mask, val, change, false); - map->unlock(map->lock_arg); - return ret; -} -EXPORT_SYMBOL_GPL(regmap_update_bits_check); +EXPORT_SYMBOL_GPL(regmap_raw_update_bits);
/** - * regmap_update_bits_check_async: Perform a read/modify/write cycle on the - * register map asynchronously and report if - * updated + * regmap_write_bits: Perform a read/modify/write cycle on the register map * * @map: Register map to update * @reg: Register to update * @mask: Bitmask to change * @val: New value for bitmask - * @change: Boolean indicating if a write was done - * - * With most buses the read must be done synchronously so this is most - * useful for devices with a cache which do not need to interact with - * the hardware to determine the current register value. * * Returns zero for success, a negative number on error. */ -int regmap_update_bits_check_async(struct regmap *map, unsigned int reg, - unsigned int mask, unsigned int val, - bool *change) +int regmap_write_bits(struct regmap *map, unsigned int reg, + unsigned int mask, unsigned int val) { int ret;
map->lock(map->lock_arg); - - map->async = true; - - ret = _regmap_update_bits(map, reg, mask, val, change, false); - - map->async = false; - + ret = _regmap_update_bits(map, reg, mask, val, NULL, true); map->unlock(map->lock_arg);
return ret; } -EXPORT_SYMBOL_GPL(regmap_update_bits_check_async); +EXPORT_SYMBOL_GPL(regmap_write_bits);
void regmap_async_complete_cb(struct regmap_async *async, int ret) { diff --git a/include/linux/regmap.h b/include/linux/regmap.h index 1839434..0b8b76a 100644 --- a/include/linux/regmap.h +++ b/include/linux/regmap.h @@ -65,6 +65,15 @@ struct reg_sequence { unsigned int delay_us; };
+#define regmap_update_bits(map, reg, mask, val) \ + regmap_raw_update_bits(map, reg, mask, val, NULL, false, false) +#define regmap_update_bits_async(map, reg, mask, val)\ + regmap_raw_update_bits(map, reg, mask, val, NULL, true, false) +#define regmap_update_bits_check(map, reg, mask, val, change)\ + regmap_raw_update_bits(map, reg, mask, val, change, false, false) +#define regmap_update_bits_check_async(map, reg, mask, val, change)\ + regmap_raw_update_bits(map, reg, mask, val, change, true, false) + #ifdef CONFIG_REGMAP
enum regmap_endian { @@ -691,18 +700,11 @@ int regmap_raw_read(struct regmap *map, unsigned int reg, void *val, size_t val_len); int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val, size_t val_count); -int regmap_update_bits(struct regmap *map, unsigned int reg, - unsigned int mask, unsigned int val); +int regmap_raw_update_bits(struct regmap *map, unsigned int reg, + unsigned int mask, unsigned int val, + bool *change, bool async, bool force); int regmap_write_bits(struct regmap *map, unsigned int reg, unsigned int mask, unsigned int val); -int regmap_update_bits_async(struct regmap *map, unsigned int reg, - unsigned int mask, unsigned int val); -int regmap_update_bits_check(struct regmap *map, unsigned int reg, - unsigned int mask, unsigned int val, - bool *change); -int regmap_update_bits_check_async(struct regmap *map, unsigned int reg, - unsigned int mask, unsigned int val, - bool *change); int regmap_get_val_bytes(struct regmap *map); int regmap_get_max_register(struct regmap *map); int regmap_get_reg_stride(struct regmap *map); @@ -937,8 +939,9 @@ static inline int regmap_bulk_read(struct regmap *map, unsigned int reg, return -EINVAL; }
-static inline int regmap_update_bits(struct regmap *map, unsigned int reg, - unsigned int mask, unsigned int val) +static inline int regmap_raw_update_bits(struct regmap *map, unsigned int reg, + unsigned int mask, unsigned int val, + bool *change, bool async, bool force) { WARN_ONCE(1, "regmap API is disabled"); return -EINVAL; @@ -951,33 +954,6 @@ static inline int regmap_write_bits(struct regmap *map, unsigned int reg, return -EINVAL; }
-static inline int regmap_update_bits_async(struct regmap *map, - unsigned int reg, - unsigned int mask, unsigned int val) -{ - WARN_ONCE(1, "regmap API is disabled"); - return -EINVAL; -} - -static inline int regmap_update_bits_check(struct regmap *map, - unsigned int reg, - unsigned int mask, unsigned int val, - bool *change) -{ - WARN_ONCE(1, "regmap API is disabled"); - return -EINVAL; -} - -static inline int regmap_update_bits_check_async(struct regmap *map, - unsigned int reg, - unsigned int mask, - unsigned int val, - bool *change) -{ - WARN_ONCE(1, "regmap API is disabled"); - return -EINVAL; -} - static inline int regmap_get_val_bytes(struct regmap *map) { WARN_ONCE(1, "regmap API is disabled");
On Wed, Feb 10, 2016 at 02:44:11AM +0000, Kuninori Morimoto wrote:
Current regmap has many similar update functions, but the difference is very few. This patch adds new regmap_raw_update_bits() and merge all update functions into it by macro.
This is a bit hard to review due to the way the diff comes out, it's not entirely clear what the code comes out looking like and I'm a bit nervous about what the gains might be since macro conversions often obscure things (this is making the macros undocumented for example). Creating the new function and then using it in a separate patch would be better.
+int regmap_raw_update_bits(struct regmap *map, unsigned int reg,
unsigned int mask, unsigned int val,bool *change, bool async, bool force)
_raw specifically means something that works with the direct physical data format within regmap, a different name would be better.
- map->async = true;
- map->async = async ? true : false;
This is abuse of the ternery operator where a normal if statement would do, and it's also rewriting a bool into a bool so a simple assignment is enough.
/**
- regmap_update_bits_check_async: Perform a read/modify/write cycle on the
register map asynchronously and report if
updated
- regmap_write_bits: Perform a read/modify/write cycle on the register map
This looks like it renames update_bits() to write_bits()...
+#define regmap_update_bits(map, reg, mask, val) \
- regmap_raw_update_bits(map, reg, mask, val, NULL, false, false)
+#define regmap_update_bits_async(map, reg, mask, val)\
- regmap_raw_update_bits(map, reg, mask, val, NULL, true, false)
+#define regmap_update_bits_check(map, reg, mask, val, change)\
- regmap_raw_update_bits(map, reg, mask, val, change, false, false)
+#define regmap_update_bits_check_async(map, reg, mask, val, change)\
- regmap_raw_update_bits(map, reg, mask, val, change, true, false)
...but we don't seem to use it? The new name is also a bit confusing.
Hi Mark
OK, I will send v2 patch soon
Current regmap has many similar update functions, but the difference is very few. This patch adds new regmap_raw_update_bits() and merge all update functions into it by macro.
This is a bit hard to review due to the way the diff comes out, it's not entirely clear what the code comes out looking like and I'm a bit nervous about what the gains might be since macro conversions often obscure things (this is making the macros undocumented for example). Creating the new function and then using it in a separate patch would be better.
+int regmap_raw_update_bits(struct regmap *map, unsigned int reg,
unsigned int mask, unsigned int val,bool *change, bool async, bool force)_raw specifically means something that works with the direct physical data format within regmap, a different name would be better.
- map->async = true;
- map->async = async ? true : false;
This is abuse of the ternery operator where a normal if statement would do, and it's also rewriting a bool into a bool so a simple assignment is enough.
/**
- regmap_update_bits_check_async: Perform a read/modify/write cycle on the
register map asynchronously and report if
updated
- regmap_write_bits: Perform a read/modify/write cycle on the register map
This looks like it renames update_bits() to write_bits()...
+#define regmap_update_bits(map, reg, mask, val) \
- regmap_raw_update_bits(map, reg, mask, val, NULL, false, false)
+#define regmap_update_bits_async(map, reg, mask, val)\
- regmap_raw_update_bits(map, reg, mask, val, NULL, true, false)
+#define regmap_update_bits_check(map, reg, mask, val, change)\
- regmap_raw_update_bits(map, reg, mask, val, change, false, false)
+#define regmap_update_bits_check_async(map, reg, mask, val, change)\
- regmap_raw_update_bits(map, reg, mask, val, change, true, false)
...but we don't seem to use it? The new name is also a bit confusing. [2 signature.asc <application/pgp-signature (7bit)>] No public key for 24D68B725D5487D0 created at 2016-02-10T18:38:15+0900 using RSA
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
It can be easy to add _check, _async, _check_async functions
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- drivers/base/regmap/regmap.c | 23 ++++++++++++++--------- include/linux/regmap.h | 14 ++++++++++---- 2 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c index c91e67b..ed2fc15 100644 --- a/drivers/base/regmap/regmap.c +++ b/drivers/base/regmap/regmap.c @@ -1690,7 +1690,7 @@ int regmap_raw_write(struct regmap *map, unsigned int reg, EXPORT_SYMBOL_GPL(regmap_raw_write);
/** - * regmap_field_write(): Write a value to a single register field + * _regmap_field_write(): Write a value to a single register field * * @field: Register field to write to * @val: Value to be written @@ -1698,12 +1698,14 @@ EXPORT_SYMBOL_GPL(regmap_raw_write); * A value of zero will be returned on success, a negative errno will * be returned in error cases. */ -int regmap_field_write(struct regmap_field *field, unsigned int val) +int _regmap_field_write(struct regmap_field *field, unsigned int val, + bool *change, bool async, bool force) { - return regmap_update_bits(field->regmap, field->reg, - field->mask, val << field->shift); + return regmap_raw_update_bits(field->regmap, field->reg, + field->mask, val << field->shift, + change, async, force); } -EXPORT_SYMBOL_GPL(regmap_field_write); +EXPORT_SYMBOL_GPL(_regmap_field_write);
/** * regmap_field_update_bits(): Perform a read/modify/write cycle @@ -1716,14 +1718,17 @@ EXPORT_SYMBOL_GPL(regmap_field_write); * A value of zero will be returned on success, a negative errno will * be returned in error cases. */ -int regmap_field_update_bits(struct regmap_field *field, unsigned int mask, unsigned int val) +int _regmap_field_update_bits(struct regmap_field *field, + unsigned int mask, unsigned int val, + bool *change, bool async, bool force) { mask = (mask << field->shift) & field->mask;
- return regmap_update_bits(field->regmap, field->reg, - mask, val << field->shift); + return regmap_raw_update_bits(field->regmap, field->reg, + mask, val << field->shift, + change, async, force); } -EXPORT_SYMBOL_GPL(regmap_field_update_bits); +EXPORT_SYMBOL_GPL(_regmap_field_update_bits);
/** * regmap_fields_write(): Write a value to a single register field with port ID diff --git a/include/linux/regmap.h b/include/linux/regmap.h index 0b8b76a..e3e54364 100644 --- a/include/linux/regmap.h +++ b/include/linux/regmap.h @@ -74,6 +74,11 @@ struct reg_sequence { #define regmap_update_bits_check_async(map, reg, mask, val, change)\ regmap_raw_update_bits(map, reg, mask, val, change, true, false)
+#define regmap_field_write(field, val) \ + _regmap_field_write(field, val, NULL, false, false) +#define regmap_field_update_bits(field, mask, val)\ + _regmap_field_update_bits(field, mask, val, NULL, false, false) + #ifdef CONFIG_REGMAP
enum regmap_endian { @@ -772,10 +777,11 @@ struct regmap_field *devm_regmap_field_alloc(struct device *dev, void devm_regmap_field_free(struct device *dev, struct regmap_field *field);
int regmap_field_read(struct regmap_field *field, unsigned int *val); -int regmap_field_write(struct regmap_field *field, unsigned int val); -int regmap_field_update_bits(struct regmap_field *field, - unsigned int mask, unsigned int val); - +int _regmap_field_write(struct regmap_field *field, unsigned int val, + bool *change, bool async, bool force); +int _regmap_field_update_bits(struct regmap_field *field, + unsigned int mask, unsigned int val, + bool *change, bool async, bool force); int regmap_fields_write(struct regmap_field *field, unsigned int id, unsigned int val); int regmap_fields_force_write(struct regmap_field *field, unsigned int id,
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
It can be easy to add _check, _async, _check_async functions
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- drivers/base/regmap/regmap.c | 32 ++++++++++++++++++-------------- include/linux/regmap.h | 15 +++++++++++---- 2 files changed, 29 insertions(+), 18 deletions(-)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c index ed2fc15..9329c27 100644 --- a/drivers/base/regmap/regmap.c +++ b/drivers/base/regmap/regmap.c @@ -1731,7 +1731,7 @@ int _regmap_field_update_bits(struct regmap_field *field, EXPORT_SYMBOL_GPL(_regmap_field_update_bits);
/** - * regmap_fields_write(): Write a value to a single register field with port ID + * _regmap_fields_write(): Write a value to a single register field with port ID * * @field: Register field to write to * @id: port ID @@ -1740,17 +1740,19 @@ EXPORT_SYMBOL_GPL(_regmap_field_update_bits); * A value of zero will be returned on success, a negative errno will * be returned in error cases. */ -int regmap_fields_write(struct regmap_field *field, unsigned int id, - unsigned int val) +int _regmap_fields_write(struct regmap_field *field, + unsigned int id, unsigned int val, + bool *change, bool async, bool force) { if (id >= field->id_size) return -EINVAL;
- return regmap_update_bits(field->regmap, - field->reg + (field->id_offset * id), - field->mask, val << field->shift); + return regmap_raw_update_bits(field->regmap, + field->reg + (field->id_offset * id), + field->mask, val << field->shift, + change, async, force); } -EXPORT_SYMBOL_GPL(regmap_fields_write); +EXPORT_SYMBOL_GPL(_regmap_fields_write);
int regmap_fields_force_write(struct regmap_field *field, unsigned int id, unsigned int val) @@ -1765,7 +1767,7 @@ int regmap_fields_force_write(struct regmap_field *field, unsigned int id, EXPORT_SYMBOL_GPL(regmap_fields_force_write);
/** - * regmap_fields_update_bits(): Perform a read/modify/write cycle + * _regmap_fields_update_bits(): Perform a read/modify/write cycle * on the register field * * @field: Register field to write to @@ -1776,19 +1778,21 @@ EXPORT_SYMBOL_GPL(regmap_fields_force_write); * A value of zero will be returned on success, a negative errno will * be returned in error cases. */ -int regmap_fields_update_bits(struct regmap_field *field, unsigned int id, - unsigned int mask, unsigned int val) +int _regmap_fields_update_bits(struct regmap_field *field, unsigned int id, + unsigned int mask, unsigned int val, + bool *change, bool async, bool force) { if (id >= field->id_size) return -EINVAL;
mask = (mask << field->shift) & field->mask;
- return regmap_update_bits(field->regmap, - field->reg + (field->id_offset * id), - mask, val << field->shift); + return regmap_raw_update_bits(field->regmap, + field->reg + (field->id_offset * id), + mask, val << field->shift, + change, async, force); } -EXPORT_SYMBOL_GPL(regmap_fields_update_bits); +EXPORT_SYMBOL_GPL(_regmap_fields_update_bits);
/* * regmap_bulk_write(): Write multiple registers to the device diff --git a/include/linux/regmap.h b/include/linux/regmap.h index e3e54364..687223d 100644 --- a/include/linux/regmap.h +++ b/include/linux/regmap.h @@ -79,6 +79,11 @@ struct reg_sequence { #define regmap_field_update_bits(field, mask, val)\ _regmap_field_update_bits(field, mask, val, NULL, false, false)
+#define regmap_fields_write(field, id, val)\ + _regmap_fields_write(field, id, val, NULL, false, false) +#define regmap_fields_update_bits(field, id, mask, val)\ + _regmap_fields_update_bits(field, id, mask, val, NULL, false, false) + #ifdef CONFIG_REGMAP
enum regmap_endian { @@ -782,14 +787,16 @@ int _regmap_field_write(struct regmap_field *field, unsigned int val, int _regmap_field_update_bits(struct regmap_field *field, unsigned int mask, unsigned int val, bool *change, bool async, bool force); -int regmap_fields_write(struct regmap_field *field, unsigned int id, - unsigned int val); +int _regmap_fields_write(struct regmap_field *field, + unsigned int id, unsigned int val, + bool *change, bool async, bool force); int regmap_fields_force_write(struct regmap_field *field, unsigned int id, unsigned int val); int regmap_fields_read(struct regmap_field *field, unsigned int id, unsigned int *val); -int regmap_fields_update_bits(struct regmap_field *field, unsigned int id, - unsigned int mask, unsigned int val); +int _regmap_fields_update_bits(struct regmap_field *field, unsigned int id, + unsigned int mask, unsigned int val, + bool *change, bool async, bool force);
/** * Description of an IRQ for the generic regmap irq_chip.
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/linux/regmap.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/linux/regmap.h b/include/linux/regmap.h index 687223d..befa781 100644 --- a/include/linux/regmap.h +++ b/include/linux/regmap.h @@ -76,8 +76,12 @@ struct reg_sequence {
#define regmap_field_write(field, val) \ _regmap_field_write(field, val, NULL, false, false) +#define regmap_field_force_write(field, val) \ + _regmap_field_write(field, val, NULL, false, true) #define regmap_field_update_bits(field, mask, val)\ _regmap_field_update_bits(field, mask, val, NULL, false, false) +#define regmap_field_force_update_bits(field, mask, val) \ + _regmap_field_update_bits(field, mask, val, NULL, false, true)
#define regmap_fields_write(field, id, val)\ _regmap_fields_write(field, id, val, NULL, false, false)
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- drivers/base/regmap/regmap.c | 12 ------------ include/linux/regmap.h | 6 ++++-- 2 files changed, 4 insertions(+), 14 deletions(-)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c index 9329c27..1b5c6e2 100644 --- a/drivers/base/regmap/regmap.c +++ b/drivers/base/regmap/regmap.c @@ -1754,18 +1754,6 @@ int _regmap_fields_write(struct regmap_field *field, } EXPORT_SYMBOL_GPL(_regmap_fields_write);
-int regmap_fields_force_write(struct regmap_field *field, unsigned int id, - unsigned int val) -{ - if (id >= field->id_size) - return -EINVAL; - - return regmap_write_bits(field->regmap, - field->reg + (field->id_offset * id), - field->mask, val << field->shift); -} -EXPORT_SYMBOL_GPL(regmap_fields_force_write); - /** * _regmap_fields_update_bits(): Perform a read/modify/write cycle * on the register field diff --git a/include/linux/regmap.h b/include/linux/regmap.h index befa781..eebee95 100644 --- a/include/linux/regmap.h +++ b/include/linux/regmap.h @@ -85,8 +85,12 @@ struct reg_sequence {
#define regmap_fields_write(field, id, val)\ _regmap_fields_write(field, id, val, NULL, false, false) +#define regmap_fields_force_write(field, id, val) \ + _regmap_fields_write(field, id, val, NULL, false, true) #define regmap_fields_update_bits(field, id, mask, val)\ _regmap_fields_update_bits(field, id, mask, val, NULL, false, false) +#define regmap_fields_force_update_bits(field, id, mask, val) \ + _regmap_fields_update_bits(field, id, mask, val, NULL, false, true)
#ifdef CONFIG_REGMAP
@@ -794,8 +798,6 @@ int _regmap_field_update_bits(struct regmap_field *field, int _regmap_fields_write(struct regmap_field *field, unsigned int id, unsigned int val, bool *change, bool async, bool force); -int regmap_fields_force_write(struct regmap_field *field, unsigned int id, - unsigned int val); int regmap_fields_read(struct regmap_field *field, unsigned int id, unsigned int *val); int _regmap_fields_update_bits(struct regmap_field *field, unsigned int id,
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
We can use _force_ write by regmap_raw_update_bits() option. Let's remove unused regmap_write_bits()
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- drivers/base/regmap/regmap.c | 23 ----------------------- include/linux/regmap.h | 9 --------- 2 files changed, 32 deletions(-)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c index 1b5c6e2..289a10e 100644 --- a/drivers/base/regmap/regmap.c +++ b/drivers/base/regmap/regmap.c @@ -2682,29 +2682,6 @@ int regmap_raw_update_bits(struct regmap *map, unsigned int reg, } EXPORT_SYMBOL_GPL(regmap_raw_update_bits);
-/** - * regmap_write_bits: Perform a read/modify/write cycle on the register map - * - * @map: Register map to update - * @reg: Register to update - * @mask: Bitmask to change - * @val: New value for bitmask - * - * Returns zero for success, a negative number on error. - */ -int regmap_write_bits(struct regmap *map, unsigned int reg, - unsigned int mask, unsigned int val) -{ - int ret; - - map->lock(map->lock_arg); - ret = _regmap_update_bits(map, reg, mask, val, NULL, true); - map->unlock(map->lock_arg); - - return ret; -} -EXPORT_SYMBOL_GPL(regmap_write_bits); - void regmap_async_complete_cb(struct regmap_async *async, int ret) { struct regmap *map = async->map; diff --git a/include/linux/regmap.h b/include/linux/regmap.h index eebee95..7724c21 100644 --- a/include/linux/regmap.h +++ b/include/linux/regmap.h @@ -721,8 +721,6 @@ int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val, int regmap_raw_update_bits(struct regmap *map, unsigned int reg, unsigned int mask, unsigned int val, bool *change, bool async, bool force); -int regmap_write_bits(struct regmap *map, unsigned int reg, - unsigned int mask, unsigned int val); int regmap_get_val_bytes(struct regmap *map); int regmap_get_max_register(struct regmap *map); int regmap_get_reg_stride(struct regmap *map); @@ -966,13 +964,6 @@ static inline int regmap_raw_update_bits(struct regmap *map, unsigned int reg, return -EINVAL; }
-static inline int regmap_write_bits(struct regmap *map, unsigned int reg, - unsigned int mask, unsigned int val) -{ - WARN_ONCE(1, "regmap API is disabled"); - return -EINVAL; -} - static inline int regmap_get_val_bytes(struct regmap *map) { WARN_ONCE(1, "regmap API is disabled");
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Some R-Car sound requests picky register access which needs *force* register write. To reduce complexity, this patch uses regmap force function for all register access.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/rcar/gen.c | 21 ++------------------- sound/soc/sh/rcar/rsnd.h | 2 -- 2 files changed, 2 insertions(+), 21 deletions(-)
diff --git a/sound/soc/sh/rcar/gen.c b/sound/soc/sh/rcar/gen.c index ea24247..a936c4b 100644 --- a/sound/soc/sh/rcar/gen.c +++ b/sound/soc/sh/rcar/gen.c @@ -104,23 +104,6 @@ void rsnd_write(struct rsnd_priv *priv, if (!rsnd_is_accessible_reg(priv, gen, reg)) return;
- regmap_fields_write(gen->regs[reg], rsnd_mod_id(mod), data); - - dev_dbg(dev, "w %s[%d] - %-18s (%4d) : %08x\n", - rsnd_mod_name(mod), rsnd_mod_id(mod), - rsnd_reg_name(gen, reg), reg, data); -} - -void rsnd_force_write(struct rsnd_priv *priv, - struct rsnd_mod *mod, - enum rsnd_reg reg, u32 data) -{ - struct device *dev = rsnd_priv_to_dev(priv); - struct rsnd_gen *gen = rsnd_priv_to_gen(priv); - - if (!rsnd_is_accessible_reg(priv, gen, reg)) - return; - regmap_fields_force_write(gen->regs[reg], rsnd_mod_id(mod), data);
dev_dbg(dev, "w %s[%d] - %-18s (%4d) : %08x\n", @@ -137,8 +120,8 @@ void rsnd_bset(struct rsnd_priv *priv, struct rsnd_mod *mod, if (!rsnd_is_accessible_reg(priv, gen, reg)) return;
- regmap_fields_update_bits(gen->regs[reg], rsnd_mod_id(mod), - mask, data); + regmap_fields_force_update_bits(gen->regs[reg], + rsnd_mod_id(mod), mask, data);
dev_dbg(dev, "b %s[%d] - %-18s (%4d) : %08x/%08x\n", rsnd_mod_name(mod), rsnd_mod_id(mod), diff --git a/sound/soc/sh/rcar/rsnd.h b/sound/soc/sh/rcar/rsnd.h index 317dd79..b536a12 100644 --- a/sound/soc/sh/rcar/rsnd.h +++ b/sound/soc/sh/rcar/rsnd.h @@ -147,8 +147,6 @@ struct rsnd_dai_stream; rsnd_read(rsnd_mod_to_priv(m), m, RSND_REG_##r) #define rsnd_mod_write(m, r, d) \ rsnd_write(rsnd_mod_to_priv(m), m, RSND_REG_##r, d) -#define rsnd_mod_force_write(m, r, d) \ - rsnd_force_write(rsnd_mod_to_priv(m), m, RSND_REG_##r, d) #define rsnd_mod_bset(m, r, s, d) \ rsnd_bset(rsnd_mod_to_priv(m), m, RSND_REG_##r, s, d)
The patch
ASoC: rsnd: rsnd_write() / rsnd_bset() uses regmap _force_ function
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 6be2553117b84e6d3e502b0af7c189ddeb89fa5c Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Mon, 15 Feb 2016 05:26:51 +0000 Subject: [PATCH] ASoC: rsnd: rsnd_write() / rsnd_bset() uses regmap _force_ function
Some R-Car sound requests picky register access which needs *force* register write. Some status register needs to set 1 to clear status, but we might read 1 from its register. In such case, current regmap does nothing and driver will be forever loop To reduce code complexity, this patch uses regmap _force_ function for all register access.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/sh/rcar/gen.c | 21 ++------------------- sound/soc/sh/rcar/rsnd.h | 2 -- 2 files changed, 2 insertions(+), 21 deletions(-)
diff --git a/sound/soc/sh/rcar/gen.c b/sound/soc/sh/rcar/gen.c index 271d29adac68..46c0ba7b6414 100644 --- a/sound/soc/sh/rcar/gen.c +++ b/sound/soc/sh/rcar/gen.c @@ -104,23 +104,6 @@ void rsnd_write(struct rsnd_priv *priv, if (!rsnd_is_accessible_reg(priv, gen, reg)) return;
- regmap_fields_write(gen->regs[reg], rsnd_mod_id(mod), data); - - dev_dbg(dev, "w %s[%d] - %-18s (%4d) : %08x\n", - rsnd_mod_name(mod), rsnd_mod_id(mod), - rsnd_reg_name(gen, reg), reg, data); -} - -void rsnd_force_write(struct rsnd_priv *priv, - struct rsnd_mod *mod, - enum rsnd_reg reg, u32 data) -{ - struct device *dev = rsnd_priv_to_dev(priv); - struct rsnd_gen *gen = rsnd_priv_to_gen(priv); - - if (!rsnd_is_accessible_reg(priv, gen, reg)) - return; - regmap_fields_force_write(gen->regs[reg], rsnd_mod_id(mod), data);
dev_dbg(dev, "w %s[%d] - %-18s (%4d) : %08x\n", @@ -137,8 +120,8 @@ void rsnd_bset(struct rsnd_priv *priv, struct rsnd_mod *mod, if (!rsnd_is_accessible_reg(priv, gen, reg)) return;
- regmap_fields_update_bits(gen->regs[reg], rsnd_mod_id(mod), - mask, data); + regmap_fields_force_update_bits(gen->regs[reg], + rsnd_mod_id(mod), mask, data);
dev_dbg(dev, "b %s[%d] - %-18s (%4d) : %08x/%08x\n", rsnd_mod_name(mod), rsnd_mod_id(mod), diff --git a/sound/soc/sh/rcar/rsnd.h b/sound/soc/sh/rcar/rsnd.h index 5f613eb42614..305cc086a0bc 100644 --- a/sound/soc/sh/rcar/rsnd.h +++ b/sound/soc/sh/rcar/rsnd.h @@ -182,8 +182,6 @@ struct rsnd_dai_stream; rsnd_read(rsnd_mod_to_priv(m), m, RSND_REG_##r) #define rsnd_mod_write(m, r, d) \ rsnd_write(rsnd_mod_to_priv(m), m, RSND_REG_##r, d) -#define rsnd_mod_force_write(m, r, d) \ - rsnd_force_write(rsnd_mod_to_priv(m), m, RSND_REG_##r, d) #define rsnd_mod_bset(m, r, s, d) \ rsnd_bset(rsnd_mod_to_priv(m), m, RSND_REG_##r, s, d)
participants (2)
-
Kuninori Morimoto -
Mark Brown