subject:"Re\: \[PATCH v3 3\/5\] mfd\: tps65090\: Stop caching most registers"

Re: [PATCH v3 3/5] mfd: tps65090: Stop caching most registers

2014-04-23 Thread Lee Jones

Anton,

 Nearly all of the registers in tps65090 combine control bits and
 status bits.  Turn off caching of all registers except the select few
 that can be cached.
 
 In order to avoid adding more duplicate #defines, we also move some
 register offset definitions to the mfd driver (and resolve
 inconsistent names).
 
 Signed-off-by: Doug Anderson diand...@chromium.org
 ---
 Changes in v3: None
 Changes in v2:
 - Leave cache on for the registers that can be cached.
 - Move register offsets to mfd header file.
 
  drivers/mfd/tps65090.c   | 27 ++-
  drivers/power/tps65090-charger.c | 11 ---

This patch still requires your Ack.

  include/linux/mfd/tps65090.h | 14 ++
  3 files changed, 28 insertions(+), 24 deletions(-)
 
 diff --git a/drivers/mfd/tps65090.c b/drivers/mfd/tps65090.c
 index c3cddb4..1c3e6e2 100644
 --- a/drivers/mfd/tps65090.c
 +++ b/drivers/mfd/tps65090.c
 @@ -32,14 +32,6 @@
  #define NUM_INT_REG 2
  #define TOTAL_NUM_REG 0x18
  
 -/* interrupt status registers */
 -#define TPS65090_INT_STS 0x0
 -#define TPS65090_INT_STS20x1
 -
 -/* interrupt mask registers */
 -#define TPS65090_INT_MSK 0x2
 -#define TPS65090_INT_MSK20x3
 -
  #define TPS65090_INT1_MASK_VAC_STATUS_CHANGE 1
  #define TPS65090_INT1_MASK_VSYS_STATUS_CHANGE2
  #define TPS65090_INT1_MASK_BAT_STATUS_CHANGE 3
 @@ -144,17 +136,26 @@ static struct regmap_irq_chip tps65090_irq_chip = {
   .irqs = tps65090_irqs,
   .num_irqs = ARRAY_SIZE(tps65090_irqs),
   .num_regs = NUM_INT_REG,
 - .status_base = TPS65090_INT_STS,
 - .mask_base = TPS65090_INT_MSK,
 + .status_base = TPS65090_REG_INTR_STS,
 + .mask_base = TPS65090_REG_INTR_MASK,
   .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
 + /* Nearly all registers have status bits mixed in, except a few */
 + switch (reg) {
 + case TPS65090_REG_INTR_MASK:
 + case TPS65090_REG_INTR_MASK2:
 + case TPS65090_REG_CG_CTRL0:
 + case TPS65090_REG_CG_CTRL1:
 + case TPS65090_REG_CG_CTRL2:
 + case TPS65090_REG_CG_CTRL3:
 + case TPS65090_REG_CG_CTRL4:
 + case TPS65090_REG_CG_CTRL5:
   return false;
 + }
 + return true;
  }
  
  static const struct regmap_config tps65090_regmap_config = {
 diff --git a/drivers/power/tps65090-charger.c 
 b/drivers/power/tps65090-charger.c
 index cc26c12..31a3ba4 100644
 --- a/drivers/power/tps65090-charger.c
 +++ b/drivers/power/tps65090-charger.c
 @@ -30,17 +30,6 @@
  
  #include linux/mfd/tps65090.h
  
 -#define TPS65090_REG_INTR_STS0x00
 -#define TPS65090_REG_INTR_MASK   0x02
 -#define TPS65090_REG_CG_CTRL00x04
 -#define TPS65090_REG_CG_CTRL10x05
 -#define TPS65090_REG_CG_CTRL20x06
 -#define TPS65090_REG_CG_CTRL30x07
 -#define TPS65090_REG_CG_CTRL40x08
 -#define TPS65090_REG_CG_CTRL50x09
 -#define TPS65090_REG_CG_STATUS1  0x0a
 -#define TPS65090_REG_CG_STATUS2  0x0b
 -
  #define TPS65090_CHARGER_ENABLE  BIT(0)
  #define TPS65090_VACGBIT(1)
  #define TPS65090_NOITERM BIT(5)
 diff --git a/include/linux/mfd/tps65090.h b/include/linux/mfd/tps65090.h
 index 3f43069..45f0f9d 100644
 --- a/include/linux/mfd/tps65090.h
 +++ b/include/linux/mfd/tps65090.h
 @@ -64,6 +64,20 @@ enum {
   TPS65090_REGULATOR_MAX,
  };
  
 +/* Register addresses */
 +#define TPS65090_REG_INTR_STS0x00
 +#define TPS65090_REG_INTR_STS2   0x01
 +#define TPS65090_REG_INTR_MASK   0x02
 +#define TPS65090_REG_INTR_MASK2  0x03
 +#define TPS65090_REG_CG_CTRL00x04
 +#define TPS65090_REG_CG_CTRL10x05
 +#define TPS65090_REG_CG_CTRL20x06
 +#define TPS65090_REG_CG_CTRL30x07
 +#define TPS65090_REG_CG_CTRL40x08
 +#define TPS65090_REG_CG_CTRL50x09
 +#define TPS65090_REG_CG_STATUS1  0x0a
 +#define TPS65090_REG_CG_STATUS2  0x0b
 +
  struct tps65090 {
   struct device   *dev;
   struct regmap   *rmap;

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/5] mfd: tps65090: Stop caching most registers

2014-04-23 Thread Lee Jones

 Nearly all of the registers in tps65090 combine control bits and
 status bits.  Turn off caching of all registers except the select few
 that can be cached.
 
 In order to avoid adding more duplicate #defines, we also move some
 register offset definitions to the mfd driver (and resolve
 inconsistent names).
 
 Signed-off-by: Doug Anderson diand...@chromium.org
 ---
 Changes in v3: None
 Changes in v2:
 - Leave cache on for the registers that can be cached.
 - Move register offsets to mfd header file.
 
  drivers/mfd/tps65090.c   | 27 ++-
  drivers/power/tps65090-charger.c | 11 ---
  include/linux/mfd/tps65090.h | 14 ++
  3 files changed, 28 insertions(+), 24 deletions(-)

Applied now, thanks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/5] mfd: tps65090: Stop caching most registers

2014-04-17 Thread Lee Jones

 Nearly all of the registers in tps65090 combine control bits and
 status bits.  Turn off caching of all registers except the select few
 that can be cached.
 
 In order to avoid adding more duplicate #defines, we also move some
 register offset definitions to the mfd driver (and resolve
 inconsistent names).
 
 Signed-off-by: Doug Anderson diand...@chromium.org
 ---
 Changes in v3: None
 Changes in v2:
 - Leave cache on for the registers that can be cached.
 - Move register offsets to mfd header file.
 
  drivers/mfd/tps65090.c   | 27 ++-
  drivers/power/tps65090-charger.c | 11 ---
  include/linux/mfd/tps65090.h | 14 ++
  3 files changed, 28 insertions(+), 24 deletions(-)
 
 diff --git a/drivers/mfd/tps65090.c b/drivers/mfd/tps65090.c
 index c3cddb4..1c3e6e2 100644
 --- a/drivers/mfd/tps65090.c
 +++ b/drivers/mfd/tps65090.c
 @@ -32,14 +32,6 @@
  #define NUM_INT_REG 2
  #define TOTAL_NUM_REG 0x18
  
 -/* interrupt status registers */
 -#define TPS65090_INT_STS 0x0
 -#define TPS65090_INT_STS20x1
 -
 -/* interrupt mask registers */
 -#define TPS65090_INT_MSK 0x2
 -#define TPS65090_INT_MSK20x3
 -
  #define TPS65090_INT1_MASK_VAC_STATUS_CHANGE 1
  #define TPS65090_INT1_MASK_VSYS_STATUS_CHANGE2
  #define TPS65090_INT1_MASK_BAT_STATUS_CHANGE 3
 @@ -144,17 +136,26 @@ static struct regmap_irq_chip tps65090_irq_chip = {
   .irqs = tps65090_irqs,
   .num_irqs = ARRAY_SIZE(tps65090_irqs),
   .num_regs = NUM_INT_REG,
 - .status_base = TPS65090_INT_STS,
 - .mask_base = TPS65090_INT_MSK,
 + .status_base = TPS65090_REG_INTR_STS,
 + .mask_base = TPS65090_REG_INTR_MASK,
   .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
 + /* Nearly all registers have status bits mixed in, except a few */
 + switch (reg) {
 + case TPS65090_REG_INTR_MASK:
 + case TPS65090_REG_INTR_MASK2:
 + case TPS65090_REG_CG_CTRL0:
 + case TPS65090_REG_CG_CTRL1:
 + case TPS65090_REG_CG_CTRL2:
 + case TPS65090_REG_CG_CTRL3:
 + case TPS65090_REG_CG_CTRL4:
 + case TPS65090_REG_CG_CTRL5:
   return false;
 + }
 + return true;
  }

I'll not force the issue, but if you wanted to do this more succinctly
you could also do:

  case TPS65090_REG_INTR_MASK ... TPS65090_REG_INTR_MASK:
  case TPS65090_REG_CG_CTRL0  ... TPS65090_REG_CG_CTRL5:

or

  if (reg = TPS65090_REG_INTR_MASK  reg = TPS65090_REG_CG_CTRL5)

Ect.

Otherwise patch looks fine:
  Acked-by: Lee Jones lee.jo...@linaro.org

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/5] mfd: tps65090: Stop caching most registers

2014-04-17 Thread Doug Anderson

Lee,

On Thu, Apr 17, 2014 at 4:00 AM, Lee Jones lee.jo...@linaro.org wrote:
 Nearly all of the registers in tps65090 combine control bits and
 status bits.  Turn off caching of all registers except the select few
 that can be cached.

 In order to avoid adding more duplicate #defines, we also move some
 register offset definitions to the mfd driver (and resolve
 inconsistent names).

 Signed-off-by: Doug Anderson diand...@chromium.org
 ---
 Changes in v3: None
 Changes in v2:
 - Leave cache on for the registers that can be cached.
 - Move register offsets to mfd header file.

  drivers/mfd/tps65090.c   | 27 ++-
  drivers/power/tps65090-charger.c | 11 ---
  include/linux/mfd/tps65090.h | 14 ++
  3 files changed, 28 insertions(+), 24 deletions(-)

 diff --git a/drivers/mfd/tps65090.c b/drivers/mfd/tps65090.c
 index c3cddb4..1c3e6e2 100644
 --- a/drivers/mfd/tps65090.c
 +++ b/drivers/mfd/tps65090.c
 @@ -32,14 +32,6 @@
  #define NUM_INT_REG 2
  #define TOTAL_NUM_REG 0x18

 -/* interrupt status registers */
 -#define TPS65090_INT_STS 0x0
 -#define TPS65090_INT_STS20x1
 -
 -/* interrupt mask registers */
 -#define TPS65090_INT_MSK 0x2
 -#define TPS65090_INT_MSK20x3
 -
  #define TPS65090_INT1_MASK_VAC_STATUS_CHANGE 1
  #define TPS65090_INT1_MASK_VSYS_STATUS_CHANGE2
  #define TPS65090_INT1_MASK_BAT_STATUS_CHANGE 3
 @@ -144,17 +136,26 @@ static struct regmap_irq_chip tps65090_irq_chip = {
   .irqs = tps65090_irqs,
   .num_irqs = ARRAY_SIZE(tps65090_irqs),
   .num_regs = NUM_INT_REG,
 - .status_base = TPS65090_INT_STS,
 - .mask_base = TPS65090_INT_MSK,
 + .status_base = TPS65090_REG_INTR_STS,
 + .mask_base = TPS65090_REG_INTR_MASK,
   .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
 + /* Nearly all registers have status bits mixed in, except a few */
 + switch (reg) {
 + case TPS65090_REG_INTR_MASK:
 + case TPS65090_REG_INTR_MASK2:
 + case TPS65090_REG_CG_CTRL0:
 + case TPS65090_REG_CG_CTRL1:
 + case TPS65090_REG_CG_CTRL2:
 + case TPS65090_REG_CG_CTRL3:
 + case TPS65090_REG_CG_CTRL4:
 + case TPS65090_REG_CG_CTRL5:
   return false;
 + }
 + return true;
  }

 I'll not force the issue, but if you wanted to do this more succinctly
 you could also do:

   case TPS65090_REG_INTR_MASK ... TPS65090_REG_INTR_MASK:
   case TPS65090_REG_CG_CTRL0  ... TPS65090_REG_CG_CTRL5:

 or

   if (reg = TPS65090_REG_INTR_MASK  reg = TPS65090_REG_CG_CTRL5)

 Ect.

 Otherwise patch looks fine:
   Acked-by: Lee Jones lee.jo...@linaro.org

If I need to spin the series for another reason I'll make that change.
 Otherwise I won't plan to spin.  Thanks for the review!

-Doug
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/5] mfd: tps65090: Stop caching most registers

Re: [PATCH v3 3/5] mfd: tps65090: Stop caching most registers

Re: [PATCH v3 3/5] mfd: tps65090: Stop caching most registers

Re: [PATCH v3 3/5] mfd: tps65090: Stop caching most registers

4 matches

Site Navigation

Mail list logo

Footer information