diff mbox

[1/5] mfd: tps65218: make INT[12] and STATUS registers volatile

Message ID 1419622104-25812-2-git-send-email-balbi@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Felipe Balbi Dec. 26, 2014, 7:28 p.m. UTC
STATUS register can be modified by the HW, so we
should bypass cache because of that.

In the case of INT[12] registers, they are the ones
that actually clear the IRQ source at the time they
are read. If we rely on the cache for them, we will
never be able to clear the interrupt, which will cause
our IRQ line to be disabled due to IRQ throttling.

Fixes: 44b4dc6 mfd: tps65218: Add driver for the TPS65218 PMIC
Cc: <stable@vger.kernel.org> # v3.15+
Cc: Keerthy <j-keerthy@ti.com>
Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/mfd/tps65218.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Felipe Balbi Jan. 6, 2015, 5:37 p.m. UTC | #1
On Fri, Dec 26, 2014 at 01:28:20PM -0600, Felipe Balbi wrote:
> STATUS register can be modified by the HW, so we
> should bypass cache because of that.
> 
> In the case of INT[12] registers, they are the ones
> that actually clear the IRQ source at the time they
> are read. If we rely on the cache for them, we will
> never be able to clear the interrupt, which will cause
> our IRQ line to be disabled due to IRQ throttling.
> 
> Fixes: 44b4dc6 mfd: tps65218: Add driver for the TPS65218 PMIC
> Cc: <stable@vger.kernel.org> # v3.15+
> Cc: Keerthy <j-keerthy@ti.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Felipe Balbi <balbi@ti.com>

ping

> ---
>  drivers/mfd/tps65218.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/mfd/tps65218.c b/drivers/mfd/tps65218.c
> index 0d256cb..2243f75 100644
> --- a/drivers/mfd/tps65218.c
> +++ b/drivers/mfd/tps65218.c
> @@ -125,10 +125,21 @@ int tps65218_clear_bits(struct tps65218 *tps, unsigned int reg,
>  }
>  EXPORT_SYMBOL_GPL(tps65218_clear_bits);
>  
> +static const struct regmap_range tps65218_yes_ranges[] = {
> +	regmap_reg_range(TPS65218_REG_INT1, TPS65218_REG_INT2),
> +	regmap_reg_range(TPS65218_REG_STATUS, TPS65218_REG_STATUS),
> +};
> +
> +static const struct regmap_access_table tps65218_volatile_table = {
> +	.yes_ranges = tps65218_yes_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(tps65218_yes_ranges),
> +};
> +
>  static struct regmap_config tps65218_regmap_config = {
>  	.reg_bits = 8,
>  	.val_bits = 8,
>  	.cache_type = REGCACHE_RBTREE,
> +	.volatile_table = &tps65218_volatile_table,
>  };
>  
>  static const struct regmap_irq tps65218_irqs[] = {
> -- 
> 2.2.0
>
Felipe Balbi Jan. 8, 2015, 4:25 p.m. UTC | #2
On Tue, Jan 06, 2015 at 11:37:34AM -0600, Felipe Balbi wrote:
> On Fri, Dec 26, 2014 at 01:28:20PM -0600, Felipe Balbi wrote:
> > STATUS register can be modified by the HW, so we
> > should bypass cache because of that.
> > 
> > In the case of INT[12] registers, they are the ones
> > that actually clear the IRQ source at the time they
> > are read. If we rely on the cache for them, we will
> > never be able to clear the interrupt, which will cause
> > our IRQ line to be disabled due to IRQ throttling.
> > 
> > Fixes: 44b4dc6 mfd: tps65218: Add driver for the TPS65218 PMIC
> > Cc: <stable@vger.kernel.org> # v3.15+
> > Cc: Keerthy <j-keerthy@ti.com>
> > Cc: Lee Jones <lee.jones@linaro.org>
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> 
> ping

another ping. Without this and the following patch TPS65218 power button
driver which was already applied by Dmitry, won't work.

> > ---
> >  drivers/mfd/tps65218.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/drivers/mfd/tps65218.c b/drivers/mfd/tps65218.c
> > index 0d256cb..2243f75 100644
> > --- a/drivers/mfd/tps65218.c
> > +++ b/drivers/mfd/tps65218.c
> > @@ -125,10 +125,21 @@ int tps65218_clear_bits(struct tps65218 *tps, unsigned int reg,
> >  }
> >  EXPORT_SYMBOL_GPL(tps65218_clear_bits);
> >  
> > +static const struct regmap_range tps65218_yes_ranges[] = {
> > +	regmap_reg_range(TPS65218_REG_INT1, TPS65218_REG_INT2),
> > +	regmap_reg_range(TPS65218_REG_STATUS, TPS65218_REG_STATUS),
> > +};
> > +
> > +static const struct regmap_access_table tps65218_volatile_table = {
> > +	.yes_ranges = tps65218_yes_ranges,
> > +	.n_yes_ranges = ARRAY_SIZE(tps65218_yes_ranges),
> > +};
> > +
> >  static struct regmap_config tps65218_regmap_config = {
> >  	.reg_bits = 8,
> >  	.val_bits = 8,
> >  	.cache_type = REGCACHE_RBTREE,
> > +	.volatile_table = &tps65218_volatile_table,
> >  };
> >  
> >  static const struct regmap_irq tps65218_irqs[] = {
> > -- 
> > 2.2.0
> > 
> 
> -- 
> balbi
Felipe Balbi Jan. 12, 2015, 4:46 p.m. UTC | #3
Hi,

On Thu, Jan 08, 2015 at 10:25:12AM -0600, Felipe Balbi wrote:
> On Tue, Jan 06, 2015 at 11:37:34AM -0600, Felipe Balbi wrote:
> > On Fri, Dec 26, 2014 at 01:28:20PM -0600, Felipe Balbi wrote:
> > > STATUS register can be modified by the HW, so we
> > > should bypass cache because of that.
> > > 
> > > In the case of INT[12] registers, they are the ones
> > > that actually clear the IRQ source at the time they
> > > are read. If we rely on the cache for them, we will
> > > never be able to clear the interrupt, which will cause
> > > our IRQ line to be disabled due to IRQ throttling.
> > > 
> > > Fixes: 44b4dc6 mfd: tps65218: Add driver for the TPS65218 PMIC
> > > Cc: <stable@vger.kernel.org> # v3.15+
> > > Cc: Keerthy <j-keerthy@ti.com>
> > > Cc: Lee Jones <lee.jones@linaro.org>
> > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > 
> > ping
> 
> another ping. Without this and the following patch TPS65218 power button
> driver which was already applied by Dmitry, won't work.

Anybody ? -rc4 is out and tps65218 is still broken because nobody has
acted on these two patches. All other patches which are meant for 3.20
merge window are applied and because of these pending, those patches
won't work.

> > > ---
> > >  drivers/mfd/tps65218.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > > 
> > > diff --git a/drivers/mfd/tps65218.c b/drivers/mfd/tps65218.c
> > > index 0d256cb..2243f75 100644
> > > --- a/drivers/mfd/tps65218.c
> > > +++ b/drivers/mfd/tps65218.c
> > > @@ -125,10 +125,21 @@ int tps65218_clear_bits(struct tps65218 *tps, unsigned int reg,
> > >  }
> > >  EXPORT_SYMBOL_GPL(tps65218_clear_bits);
> > >  
> > > +static const struct regmap_range tps65218_yes_ranges[] = {
> > > +	regmap_reg_range(TPS65218_REG_INT1, TPS65218_REG_INT2),
> > > +	regmap_reg_range(TPS65218_REG_STATUS, TPS65218_REG_STATUS),
> > > +};
> > > +
> > > +static const struct regmap_access_table tps65218_volatile_table = {
> > > +	.yes_ranges = tps65218_yes_ranges,
> > > +	.n_yes_ranges = ARRAY_SIZE(tps65218_yes_ranges),
> > > +};
> > > +
> > >  static struct regmap_config tps65218_regmap_config = {
> > >  	.reg_bits = 8,
> > >  	.val_bits = 8,
> > >  	.cache_type = REGCACHE_RBTREE,
> > > +	.volatile_table = &tps65218_volatile_table,
> > >  };
> > >  
> > >  static const struct regmap_irq tps65218_irqs[] = {
> > > -- 
> > > 2.2.0
> > > 
> > 
> > -- 
> > balbi
> 
> 
> 
> -- 
> balbi
Tony Lindgren Jan. 14, 2015, 5:07 p.m. UTC | #4
* Felipe Balbi <balbi@ti.com> [150112 08:50]:
> Hi,
> 
> On Thu, Jan 08, 2015 at 10:25:12AM -0600, Felipe Balbi wrote:
> > On Tue, Jan 06, 2015 at 11:37:34AM -0600, Felipe Balbi wrote:
> > > On Fri, Dec 26, 2014 at 01:28:20PM -0600, Felipe Balbi wrote:
> > > > STATUS register can be modified by the HW, so we
> > > > should bypass cache because of that.
> > > > 
> > > > In the case of INT[12] registers, they are the ones
> > > > that actually clear the IRQ source at the time they
> > > > are read. If we rely on the cache for them, we will
> > > > never be able to clear the interrupt, which will cause
> > > > our IRQ line to be disabled due to IRQ throttling.
> > > > 
> > > > Fixes: 44b4dc6 mfd: tps65218: Add driver for the TPS65218 PMIC
> > > > Cc: <stable@vger.kernel.org> # v3.15+
> > > > Cc: Keerthy <j-keerthy@ti.com>
> > > > Cc: Lee Jones <lee.jones@linaro.org>
> > > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > > 
> > > ping
> > 
> > another ping. Without this and the following patch TPS65218 power button
> > driver which was already applied by Dmitry, won't work.
> 
> Anybody ? -rc4 is out and tps65218 is still broken because nobody has
> acted on these two patches. All other patches which are meant for 3.20
> merge window are applied and because of these pending, those patches
> won't work.

Lee, are you planning to pick these two drivers/mfd patches?

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Felipe Balbi Jan. 16, 2015, 10:57 p.m. UTC | #5
Hi,

On Wed, Jan 14, 2015 at 09:07:17AM -0800, Tony Lindgren wrote:
> > > > > STATUS register can be modified by the HW, so we
> > > > > should bypass cache because of that.
> > > > > 
> > > > > In the case of INT[12] registers, they are the ones
> > > > > that actually clear the IRQ source at the time they
> > > > > are read. If we rely on the cache for them, we will
> > > > > never be able to clear the interrupt, which will cause
> > > > > our IRQ line to be disabled due to IRQ throttling.
> > > > > 
> > > > > Fixes: 44b4dc6 mfd: tps65218: Add driver for the TPS65218 PMIC
> > > > > Cc: <stable@vger.kernel.org> # v3.15+
> > > > > Cc: Keerthy <j-keerthy@ti.com>
> > > > > Cc: Lee Jones <lee.jones@linaro.org>
> > > > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > > > 
> > > > ping
> > > 
> > > another ping. Without this and the following patch TPS65218 power button
> > > driver which was already applied by Dmitry, won't work.
> > 
> > Anybody ? -rc4 is out and tps65218 is still broken because nobody has
> > acted on these two patches. All other patches which are meant for 3.20
> > merge window are applied and because of these pending, those patches
> > won't work.
> 
> Lee, are you planning to pick these two drivers/mfd patches?

Samuel, Lee, is someone planning on picking these two patches up ?
v3.19 remains broken.
Lee Jones Jan. 18, 2015, 9:52 a.m. UTC | #6
On Fri, 26 Dec 2014, Felipe Balbi wrote:

> STATUS register can be modified by the HW, so we
> should bypass cache because of that.
> 
> In the case of INT[12] registers, they are the ones
> that actually clear the IRQ source at the time they
> are read. If we rely on the cache for them, we will
> never be able to clear the interrupt, which will cause
> our IRQ line to be disabled due to IRQ throttling.
> 
> Fixes: 44b4dc6 mfd: tps65218: Add driver for the TPS65218 PMIC
> Cc: <stable@vger.kernel.org> # v3.15+
> Cc: Keerthy <j-keerthy@ti.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
>  drivers/mfd/tps65218.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)

Sorry for the delay.  It's difficult to get a WiFi signal 2000m up in
an Austrian mountain. :)

Applied now, thanks.

> diff --git a/drivers/mfd/tps65218.c b/drivers/mfd/tps65218.c
> index 0d256cb..2243f75 100644
> --- a/drivers/mfd/tps65218.c
> +++ b/drivers/mfd/tps65218.c
> @@ -125,10 +125,21 @@ int tps65218_clear_bits(struct tps65218 *tps, unsigned int reg,
>  }
>  EXPORT_SYMBOL_GPL(tps65218_clear_bits);
>  
> +static const struct regmap_range tps65218_yes_ranges[] = {
> +	regmap_reg_range(TPS65218_REG_INT1, TPS65218_REG_INT2),
> +	regmap_reg_range(TPS65218_REG_STATUS, TPS65218_REG_STATUS),
> +};
> +
> +static const struct regmap_access_table tps65218_volatile_table = {
> +	.yes_ranges = tps65218_yes_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(tps65218_yes_ranges),
> +};
> +
>  static struct regmap_config tps65218_regmap_config = {
>  	.reg_bits = 8,
>  	.val_bits = 8,
>  	.cache_type = REGCACHE_RBTREE,
> +	.volatile_table = &tps65218_volatile_table,
>  };
>  
>  static const struct regmap_irq tps65218_irqs[] = {
Felipe Balbi Jan. 19, 2015, 2:41 p.m. UTC | #7
On Sun, Jan 18, 2015 at 09:52:14AM +0000, Lee Jones wrote:
> On Fri, 26 Dec 2014, Felipe Balbi wrote:
> 
> > STATUS register can be modified by the HW, so we
> > should bypass cache because of that.
> > 
> > In the case of INT[12] registers, they are the ones
> > that actually clear the IRQ source at the time they
> > are read. If we rely on the cache for them, we will
> > never be able to clear the interrupt, which will cause
> > our IRQ line to be disabled due to IRQ throttling.
> > 
> > Fixes: 44b4dc6 mfd: tps65218: Add driver for the TPS65218 PMIC
> > Cc: <stable@vger.kernel.org> # v3.15+
> > Cc: Keerthy <j-keerthy@ti.com>
> > Cc: Lee Jones <lee.jones@linaro.org>
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > ---
> >  drivers/mfd/tps65218.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> 
> Sorry for the delay.  It's difficult to get a WiFi signal 2000m up in
> an Austrian mountain. :)

now you're just making excuses ;-)

> Applied now, thanks.

thanks :-)
diff mbox

Patch

diff --git a/drivers/mfd/tps65218.c b/drivers/mfd/tps65218.c
index 0d256cb..2243f75 100644
--- a/drivers/mfd/tps65218.c
+++ b/drivers/mfd/tps65218.c
@@ -125,10 +125,21 @@  int tps65218_clear_bits(struct tps65218 *tps, unsigned int reg,
 }
 EXPORT_SYMBOL_GPL(tps65218_clear_bits);
 
+static const struct regmap_range tps65218_yes_ranges[] = {
+	regmap_reg_range(TPS65218_REG_INT1, TPS65218_REG_INT2),
+	regmap_reg_range(TPS65218_REG_STATUS, TPS65218_REG_STATUS),
+};
+
+static const struct regmap_access_table tps65218_volatile_table = {
+	.yes_ranges = tps65218_yes_ranges,
+	.n_yes_ranges = ARRAY_SIZE(tps65218_yes_ranges),
+};
+
 static struct regmap_config tps65218_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
 	.cache_type = REGCACHE_RBTREE,
+	.volatile_table = &tps65218_volatile_table,
 };
 
 static const struct regmap_irq tps65218_irqs[] = {