| Message ID | 1397592876-5741-3-git-send-email-dianders@chromium.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
> Nearly all of the registers in tps65090 combine control bits and > status bits. Turn off caching of registers so that we can read status > bits reliably. > > NOTE: the IRQnMASK and CG_CTRLn registers are the exception and could > be cached. If we find that we spend a lot of time reading those we > can turn on cache for just those registers. > > Signed-off-by: Doug Anderson <dianders@chromium.org> > --- > drivers/mfd/tps65090.c | 10 ---------- > 1 file changed, 10 deletions(-) > > diff --git a/drivers/mfd/tps65090.c b/drivers/mfd/tps65090.c > index c3cddb4..4cfdd07 100644 > --- a/drivers/mfd/tps65090.c > +++ b/drivers/mfd/tps65090.c > @@ -149,21 +149,11 @@ static struct regmap_irq_chip tps65090_irq_chip = { > .mask_invert = true, > }; > > -static bool is_volatile_reg(struct device *dev, unsigned int reg) > -{ > - if ((reg == TPS65090_INT_STS) || (reg == TPS65090_INT_STS2)) > - return true; > - else > - return false; > -} > - I don't know enough about Regmap internals to know what this actually affects in real terms. Mark, Does this change seem sane to you? > static const struct regmap_config tps65090_regmap_config = { > .reg_bits = 8, > .val_bits = 8, > .max_register = TOTAL_NUM_REG, > .num_reg_defaults_raw = TOTAL_NUM_REG, > - .cache_type = REGCACHE_RBTREE, > - .volatile_reg = is_volatile_reg, > }; > > #ifdef CONFIG_OF
On Wed, Apr 16, 2014 at 10:59:22AM +0100, Lee Jones wrote: > > NOTE: the IRQnMASK and CG_CTRLn registers are the exception and could > > be cached. If we find that we spend a lot of time reading those we > > can turn on cache for just those registers. > > -static bool is_volatile_reg(struct device *dev, unsigned int reg) > > -{ > > - if ((reg == TPS65090_INT_STS) || (reg == TPS65090_INT_STS2)) > > - return true; > > - else > > - return false; > > -} > > - > I don't know enough about Regmap internals to know what this actually > affects in real terms. > Mark, > Does this change seem sane to you? It does what it says, it stops us caching stuff. It would seem better to do what the changelog suggests above and keep caching the registers that can be cached - especially the interrupt masks, they should get read in the interrupt path and that tends to be a bit latency sensitive.
Mark, On Wed, Apr 16, 2014 at 3:13 AM, Mark Brown <broonie@kernel.org> wrote: > On Wed, Apr 16, 2014 at 10:59:22AM +0100, Lee Jones wrote: > >> > NOTE: the IRQnMASK and CG_CTRLn registers are the exception and could >> > be cached. If we find that we spend a lot of time reading those we >> > can turn on cache for just those registers. > >> > -static bool is_volatile_reg(struct device *dev, unsigned int reg) >> > -{ >> > - if ((reg == TPS65090_INT_STS) || (reg == TPS65090_INT_STS2)) >> > - return true; >> > - else >> > - return false; >> > -} >> > - > >> I don't know enough about Regmap internals to know what this actually >> affects in real terms. > >> Mark, >> Does this change seem sane to you? > > It does what it says, it stops us caching stuff. It would seem better > to do what the changelog suggests above and keep caching the registers > that can be cached - especially the interrupt masks, they should get > read in the interrupt path and that tends to be a bit latency sensitive. Done -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mfd/tps65090.c b/drivers/mfd/tps65090.c index c3cddb4..4cfdd07 100644 --- a/drivers/mfd/tps65090.c +++ b/drivers/mfd/tps65090.c @@ -149,21 +149,11 @@ static struct regmap_irq_chip tps65090_irq_chip = { .mask_invert = true, }; -static bool is_volatile_reg(struct device *dev, unsigned int reg) -{ - if ((reg == TPS65090_INT_STS) || (reg == TPS65090_INT_STS2)) - return true; - else - return false; -} - static const struct regmap_config tps65090_regmap_config = { .reg_bits = 8, .val_bits = 8, .max_register = TOTAL_NUM_REG, .num_reg_defaults_raw = TOTAL_NUM_REG, - .cache_type = REGCACHE_RBTREE, - .volatile_reg = is_volatile_reg, }; #ifdef CONFIG_OF
Nearly all of the registers in tps65090 combine control bits and status bits. Turn off caching of registers so that we can read status bits reliably. NOTE: the IRQnMASK and CG_CTRLn registers are the exception and could be cached. If we find that we spend a lot of time reading those we can turn on cache for just those registers. Signed-off-by: Doug Anderson <dianders@chromium.org> --- drivers/mfd/tps65090.c | 10 ---------- 1 file changed, 10 deletions(-)