Re: [PATCH V2 1/4] regmap: cache: Add "was_reset" argument to regcache_sync_region()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 04/25/2015 12:36 AM, Kevin Cernekee wrote:
regcache_sync() and regcache_sync_region() currently assume that the
hardware has just emerged from a clean reset, and that all registers are
in their default states.  But that isn't the only possibility; the device
may have been in a different state in which the registers were
inaccessible but have retained their contents, e.g. clock gating.

So we will extend the more versatile of the two functions,
regcache_sync_region(), to let the caller decide what assumptions should
be made.

One driver that can benefit from this is adau1977, which has hacks to
overwrite the registers that regcache_sync() might have missed.

The issue with the adau1977 is slightly different, there is only one single register which is not affected, so we need special handling for that register. For all other registers the default behavior of regcache_sync() is correct.

Also, the powerdown pin on tas571x does not reset the register contents
either, so a similar feature will be required by that driver.

This commit just adds the new argument by changing the function
declarations and call sites, but doesn't wire it up yet.

I think a better approach is to introduce a new flag internally, similar to the cache_dirty flag. Set both this new flag and cache_dirty when regcache_mark_dirty() is called. But only cache_dirty when a write is performed when the regmap is marked as cache only. Now in regcache_sync() and friends check this new flag and only when it is set skip registers which match the default. At the end of regcache_sync() clear both flags.

[...]
@@ -600,7 +605,8 @@ static bool regcache_reg_present(unsigned long *cache_present, unsigned int idx)
  static int regcache_sync_block_single(struct regmap *map, void *block,
  				      unsigned long *cache_present,
  				      unsigned int block_base,
-				      unsigned int start, unsigned int end)
+				      unsigned int start, unsigned int end,
+				      bool was_reset)
  {
  	unsigned int i, regtmp, val;
  	int ret;
@@ -614,10 +620,12 @@ static int regcache_sync_block_single(struct regmap *map, void *block,

  		val = regcache_get_val(map, block, i);

-		/* Is this the hardware default?  If so skip. */
-		ret = regcache_lookup_reg(map, regtmp);
-		if (ret >= 0 && val == map->reg_defaults[ret].def)
-			continue;
+		if (was_reset) {
+			/* Is this the hardware default?  If so skip. */
+			ret = regcache_lookup_reg(map, regtmp);
+			if (ret >= 0 && val == map->reg_defaults[ret].def)
+				continue;
+		}

Probably factor the check whether wee need to sync or not into a helper function, that can be used here and below in sync_block_raw().

[...]
@@ -689,14 +697,17 @@ static int regcache_sync_block_raw(struct regmap *map, void *block,

  		val = regcache_get_val(map, block, i);

-		/* Is this the hardware default?  If so skip. */
-		ret = regcache_lookup_reg(map, regtmp);
-		if (ret >= 0 && val == map->reg_defaults[ret].def) {
-			ret = regcache_sync_block_raw_flush(map, &data,
-							    base, regtmp);
-			if (ret != 0)
-				return ret;
-			continue;
+		if (was_reset) {
+			/* Is this the hardware default?  If so skip. */
+			ret = regcache_lookup_reg(map, regtmp);
+			if (ret >= 0 && val == map->reg_defaults[ret].def) {
+				ret = regcache_sync_block_raw_flush(map, &data,
+								    base,
+								    regtmp);
+				if (ret != 0)
+					return ret;
+				continue;
+			}
  		}

[...]
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux