iio: chemical: atlas-ph-sensor: switch regmap cache
PATCH 1/1 iio: chemical: atlas-ph-sensor: switch regmap cache
switch from using REGCACHE_FLAT to REGCACHE_RBTREE so initial hw values are read from device. This also allows some volatile ranges to be dropped.
--- drivers/iio/chemical/atlas-ph-sensor.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/iio/chemical/atlas-ph-sensor.c +1 -3
@@ -65,8 +65,6 @@ struct atlas_data {static const struct regmap_range atlas_volatile_ranges[] = {regmap_reg_range (ATLAS_REG_INT_CONTROL ,ATLAS_REG_INT_CONTROL ), -regmap_reg_range (ATLAS_REG_CALIB_STATUS ,ATLAS_REG_CALIB_STATUS ), -regmap_reg_range (ATLAS_REG_TEMP_DATA ,ATLAS_REG_TEMP_DATA +4 ),regmap_reg_range (ATLAS_REG_PH_DATA ,ATLAS_REG_PH_DATA +4 ), };
@@ -83,7 +81,7 @@ static const struct regmap_config atlas_regmap_config = { .volatile_table = &atlas_volatile_table, .max_register = ATLAS_REG_PH_DATA + 4, - .cache_type = REGCACHE_FLAT, + .cache_type = REGCACHE_RBTREE, };static const struct iio_chan_spec atlas_channels[] = {
Signature · 1 line
2.7.0
Jonathan Cameron
maintainerQuoted 5 lines · Matt Ranostay
switch from using
REGCACHE_FLATtoREGCACHE_RBTREEso initial hw values are read from device. This also allows some volatile ranges to be dropped.Signed-off-by Matt Ranostay <[email protected]>
I'm lost here. Why should changing the storage of the register cache result in different behaviour other than in efficiency of access to cached values?
Jonathan
Quoted 27 lines
--- drivers/iio/chemical/atlas-ph-sensor.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)drivers/iio/chemical/atlas-ph-sensor.c +1
-3@@ -65,8 +65,6 @@ struct atlas_data {static const struct regmap_range atlas_volatile_ranges[] = {regmap_reg_range (ATLAS_REG_INT_CONTROL ,ATLAS_REG_INT_CONTROL ), -regmap_reg_range (ATLAS_REG_CALIB_STATUS ,ATLAS_REG_CALIB_STATUS ), -regmap_reg_range (ATLAS_REG_TEMP_DATA ,ATLAS_REG_TEMP_DATA +4 ),regmap_reg_range (ATLAS_REG_PH_DATA ,ATLAS_REG_PH_DATA +4 ), };@@ -83,7 +81,7 @@ static const struct regmap_config atlas_regmap_config = { .volatile_table = &atlas_volatile_table, .max_register = ATLAS_REG_PH_DATA + 4, - .cache_type = REGCACHE_FLAT, + .cache_type = REGCACHE_RBTREE, };static const struct iio_chan_spec atlas_channels[] = {
Quoted 8 lines · Jonathan Cameron
Quoted 5 lines · Matt Ranostay
switch from using
REGCACHE_FLATtoREGCACHE_RBTREEso initial hw values are read from device. This also allows some volatile ranges to be dropped.Signed-off-by Matt Ranostay <[email protected]>I'm lost here. Why should changing the storage of the register cache result in different behaviour other than in efficiency of access to cached values?
And if there is really a difference this should probably be addressed at the regmap level.
- Lars
Quoted 29 lines
Jonathan
Quoted 27 lines
--- drivers/iio/chemical/atlas-ph-sensor.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)drivers/iio/chemical/atlas-ph-sensor.c +1
-3@@ -65,8 +65,6 @@ struct atlas_data {static const struct regmap_range atlas_volatile_ranges[] = {regmap_reg_range (ATLAS_REG_INT_CONTROL ,ATLAS_REG_INT_CONTROL ), -regmap_reg_range (ATLAS_REG_CALIB_STATUS ,ATLAS_REG_CALIB_STATUS ), -regmap_reg_range (ATLAS_REG_TEMP_DATA ,ATLAS_REG_TEMP_DATA +4 ),regmap_reg_range (ATLAS_REG_PH_DATA ,ATLAS_REG_PH_DATA +4 ), };@@ -83,7 +81,7 @@ static const struct regmap_config atlas_regmap_config = { .volatile_table = &atlas_volatile_table, .max_register = ATLAS_REG_PH_DATA + 4, - .cache_type = REGCACHE_FLAT, + .cache_type = REGCACHE_RBTREE, };static const struct iio_chan_spec atlas_channels[] = {
Should have CC'ed Mark Brown on the original post.
But anyway after talking to him it seems that REGCACHE_FLAT is "special" since it a flat tree it won't read it and cache the initial value, so it then defaults to zero.
Mark, thoughts?
Thanks,
Matt
Quoted 49 lines · Lars-Peter Clausen
Quoted 8 lines · Jonathan Cameron
Quoted 5 lines · Matt Ranostay
switch from using
REGCACHE_FLATtoREGCACHE_RBTREEso initial hw values are read from device. This also allows some volatile ranges to be dropped.Signed-off-by Matt Ranostay <[email protected]>I'm lost here. Why should changing the storage of the register cache result in different behaviour other than in efficiency of access to cached values?
And if there is really a difference this should probably be addressed at the regmap level.
- Lars
Quoted 21 lines
Jonathan
Quoted 19 lines
--- drivers/iio/chemical/atlas-ph-sensor.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)drivers/iio/chemical/atlas-ph-sensor.c +0
-2@@ -65,8 +65,6 @@ struct atlas_data {static const struct regmap_range atlas_volatile_ranges[] = {regmap_reg_range (ATLAS_REG_INT_CONTROL ,ATLAS_REG_INT_CONTROL ), -regmap_reg_range (ATLAS_REG_CALIB_STATUS ,ATLAS_REG_CALIB_STATUS ), -regmap_reg_range (ATLAS_REG_TEMP_DATA ,ATLAS_REG_TEMP_DATA +4 ),regmap_reg_range (ATLAS_REG_PH_DATA ,ATLAS_REG_PH_DATA +4 ), };@@ -83,7 +81,7 @@ static const struct regmap_config atlas_regmap_config = {Quoted 8 lines
drivers/iio/chemical/atlas-ph-sensor.c static const struct regmap_config atlas_regmap_config = { · line 87
Quoted 8 lines
drivers/iio/chemical/atlas-ph-sensor.c +1
-1.volatile_table = &atlas_volatile_table, .max_register = ATLAS_REG_PH_DATA + 4, - .cache_type = REGCACHE_FLAT, + .cache_type = REGCACHE_RBTREE, };static const struct iio_chan_spec atlas_channels[] = {--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 7 lines · Lars-Peter Clausen
Quoted 2 lines · Jonathan Cameron
I'm lost here. Why should changing the storage of the register cache result in different behaviour other than in efficiency of access to cached values?
And if there is really a difference this should probably be addressed at the regmap level.
You'd need to make regcache-flat keep track of which cache values are valid either during init (in which case it'd have to read every other register at that time) or at runtime (which would mean extra operations in a fast path). Flat really is special here, there's no reason to use it over a rbtree other than being very sensitive about performance.
Please also be aware that if you CC me on an iio patch series there's a good chance I'll just delete it without reading it, for some reason I keep on getting copied on lots of them which have no relevance to me that I'm able to identify.
Jonathan Cameron
maintainerQuoted 15 lines · Mark Brown
Quoted 7 lines · Lars-Peter Clausen
Quoted 2 lines · Jonathan Cameron
I'm lost here. Why should changing the storage of the register cache result in different behaviour other than in efficiency of access to cached values?
And if there is really a difference this should probably be addressed at the regmap level.
You'd need to make regcache-flat keep track of which cache values are valid either during init (in which case it'd have to read every other register at that time) or at runtime (which would mean extra operations in a fast path). Flat really is special here, there's no reason to use it over a rbtree other than being very sensitive about performance.
Quoted 5 lines
Please also be aware that if you CC me on an iio patch series there's a good chance I'll just delete it without reading it, for some reason I keep on getting copied on lots of them which have no relevance to me that I'm able to identify.
Hmm. Not the best documented 'non intuitive case' ever, but fair enough. Glad you did pick this one up Mark!
So Matt, the reason those two elements are dropped from being volatile is that they never were, but because we were using a flat cache it wasn't getting the original values?
If so fair enough I suppose, but please confirm that I've understood this correctly + I'll probably expand the patch description somewhat to make it clearer why the original was wrong.
Jonathan
Quoted 24 lines · Jonathan Cameron
Quoted 15 lines · Mark Brown
Quoted 7 lines · Lars-Peter Clausen
Quoted 2 lines · Jonathan Cameron
I'm lost here. Why should changing the storage of the register cache result in different behaviour other than in efficiency of access to cached values?
And if there is really a difference this should probably be addressed at the regmap level.
You'd need to make regcache-flat keep track of which cache values are valid either during init (in which case it'd have to read every other register at that time) or at runtime (which would mean extra operations in a fast path). Flat really is special here, there's no reason to use it over a rbtree other than being very sensitive about performance.
Quoted 5 lines
Please also be aware that if you CC me on an iio patch series there's a good chance I'll just delete it without reading it, for some reason I keep on getting copied on lots of them which have no relevance to me that I'm able to identify.
Hmm. Not the best documented 'non intuitive case' ever, but fair enough. Glad you did pick this one up Mark!
Quoted 7 lines
So Matt, the reason those two elements are dropped from being volatile is that they never were, but because we were using a flat cache it wasn't getting the original values?
If so fair enough I suppose, but please confirm that I've understood this correctly + I'll probably expand the patch description somewhat to make it clearer why the original was wrong.
That is correct. Flat cache wasn't getting the on device value without it being set as volatile.
Quoted 2 lines
Jonathan
Jonathan Cameron
maintainerQuoted 29 lines · Matt Ranostay
Quoted 28 lines · Jonathan Cameron
Quoted 15 lines · Mark Brown
Quoted 7 lines · Lars-Peter Clausen
Quoted 2 lines · Jonathan Cameron
I'm lost here. Why should changing the storage of the register cache result in different behaviour other than in efficiency of access to cached values?
And if there is really a difference this should probably be addressed at the regmap level.
You'd need to make regcache-flat keep track of which cache values are valid either during init (in which case it'd have to read every other register at that time) or at runtime (which would mean extra operations in a fast path). Flat really is special here, there's no reason to use it over a rbtree other than being very sensitive about performance.
Quoted 5 lines
Please also be aware that if you CC me on an iio patch series there's a good chance I'll just delete it without reading it, for some reason I keep on getting copied on lots of them which have no relevance to me that I'm able to identify.
Hmm. Not the best documented 'non intuitive case' ever, but fair enough. Glad you did pick this one up Mark!
So Matt, the reason those two elements are dropped from being volatile is that they never were, but because we were using a flat cache it wasn't getting the original values?
Quoted 6 lines
Quoted 3 lines
If so fair enough I suppose, but please confirm that I've understood this correctly + I'll probably expand the patch description somewhat to make it clearer why the original was wrong.
That is correct. Flat cache wasn't getting the on device value without it being set as volatile.
Applied to the togreg branch iio.git - initially pushed out as testing for the autobuilders to play with it.
Thanks,
Jonathan
Quoted 9 lines
Quoted 4 lines
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html