Skip to contentSkip to conversation

regmap: Add reg_sequence for use with multi_reg_write / register_patch

RFC PATCH In review Nariman Poushin 2 participants 4 messages Jun 1, 2015 – Jun 4, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 regmap: Add reg_sequence for use with multi_reg_write / register_patch

Support write sequences / patches with specified delays (in microseconds) after some (or all) writes. Logically separate reg_default from the new reg_sequence structure (which has an additional delay_us member) as the reg_default tables can run in to the thousands (for modern devices) and the additional memory usage due to an added unsigned int could be large (especially on 64-bit systems).

We treat a delay in a sequence the same as we treat a page change as they are logically similar (you can coalesce all writes within the same page or if none of them require a delay after being written).

---

I am not sure how best to update all the users of this patch (should it be accepted), should I:
- Squash all the updates in to this patch (I suppose the benefit there is that we don't break the kernel build from one patch to the other)
- Provide separate patch(es) for the users

Thanks
Nariman

 drivers/base/regmap/internal.h |  2 +-
 drivers/base/regmap/regmap.c   | 56 ++++++++++++++++++++++++++++++------------
 include/linux/regmap.h         | 21 +++++++++++++---
 3 files changed, 59 insertions(+), 20 deletions(-)
drivers/base/regmap/internal.h +1 -1
index b2b2849..873ddf9 100644
@@ -136,7 +136,7 @@ struct regmap {
 	/* if set, the HW registers are known to match map->reg_defaults */
 	bool no_sync_defaults;
 
-	struct reg_default *patch;
+	struct reg_sequence *patch;
 	int patch_regs;
 
 	/* if set, converts bulk rw to single rw */
drivers/base/regmap/regmap.c +40 -16

148 lines not on this page · raw message

include/linux/regmap.h +18 -3
index 116655d..c3bf366 100644
@@ -50,6 +50,21 @@ struct reg_default {
 	unsigned int def;
 };
 
+/**
+ * Register / Value pairs for sequences of writes, incorporating an optional
+ * delay in microseconds.
+ *
+ * @reg: Register address.
+ * @def: Register default value.
+ * @delay_us: Delay in microseconds
+ */
+
+struct reg_sequence {
+	unsigned reg;
+	unsigned def;
+	unsigned delay_us;
+};
+
 #ifdef CONFIG_REGMAP
 
 enum regmap_endian {
@@ -410,10 +425,10 @@ int regmap_raw_write(struct regmap *map, unsigned int reg,
 		     const void *val, size_t val_len);
 int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 			size_t val_count);
-int regmap_multi_reg_write(struct regmap *map, const struct reg_default *regs,
+int regmap_multi_reg_write(struct regmap *map, const struct reg_sequence *regs,
 			int num_regs);
 int regmap_multi_reg_write_bypassed(struct regmap *map,
-				    const struct reg_default *regs,
+				    const struct reg_sequence *regs,
 				    int num_regs);
 int regmap_raw_write_async(struct regmap *map, unsigned int reg,
 			   const void *val, size_t val_len);
@@ -448,7 +463,7 @@ void regcache_mark_dirty(struct regmap *map);
 bool regmap_check_range_table(struct regmap *map, unsigned int reg,
 			      const struct regmap_access_table *table);
 
-int regmap_register_patch(struct regmap *map, const struct reg_default *regs,
+int regmap_register_patch(struct regmap *map, const struct reg_sequence *regs,
 			  int num_regs);
 int regmap_parse_val(struct regmap *map, const void *buf,
 				unsigned int *val);
Signature · 1 line
2.1.4
maintainer
#1 lore raw
Quoted 4 lines · Nariman Poushin

it be accepted), should I:
- Squash all the updates in to this patch (I suppose the benefit there is that we don't break the kernel build from one patch to the other)

You need to squash the changes in since they break bisection if handled separately. It would be better to do this by having a separate patch to add the newly named structure rather than adding the new functionality at the same time. That makes the patch more mechanical and easier to review.

#2 lore raw
Quoted 7 lines · Mark Brown
Quoted 4 lines · Nariman Poushin

it be accepted), should I:
- Squash all the updates in to this patch (I suppose the benefit there is that we don't break the kernel build from one patch to the other)

Quoted 5 lines

You need to squash the changes in since they break bisection if handled separately. It would be better to do this by having a separate patch to add the newly named structure rather than adding the new functionality at the same time. That makes the patch more mechanical and easier to review.

Ok, I have a patch set ready (as you described) but I am having some problem deciding on the correct distribution, the squashed patch that touches a whole bunch of subsystems ends up with a monstrous get_maintainer.pl output, so even going through and checking MAINTAINERS I have ended up with a large list (26 individuals and lists).

Is this ok? I am not sure if it is going to get bounced by mail servers as spam or whether it's bad etiquette to do this, but as you say we don't want to break the bisection.

Thanks
Nariman

maintainer
#3 lore raw
Quoted 14 lines · Nariman Poushin
Quoted 5 lines · Mark Brown

You need to squash the changes in since they break bisection if handled separately. It would be better to do this by having a separate patch to add the newly named structure rather than adding the new functionality at the same time. That makes the patch more mechanical and easier to review.

Ok, I have a patch set ready (as you described) but I am having some problem deciding on the correct distribution, the squashed patch that touches a whole bunch of subsystems ends up with a monstrous get_maintainer.pl output, so even going through and checking MAINTAINERS I have ended up with a large list (26 individuals and lists).

Quoted 3 lines

Is this ok? I am not sure if it is going to get bounced by mail servers as spam or whether it's bad etiquette to do this, but as you say we don't want to break the bisection.

Probably post individual patches for people to look at but make it clear that they need to go in as a single API change patch rather than be applied in the commit message.