Re: [PATCH v2] Input: pmic8xxx-pwrkey - Support shutdown

2015-07-13 Thread Bjorn Andersson
On Mon 13 Jul 16:30 PDT 2015, Stephen Boyd wrote:

 On pm8xxx PMICs, shutdown and restart are signaled to the PMIC
 via a pin called PS_HOLD. When this pin goes low, the PMIC
 performs a configurable power sequence. Add a .shutdown hook so
 that we can properly configure this power sequence for shutdown
 or restart depending on the system state.
 
 Cc: Bjorn Andersson bjorn.anders...@sonymobile.com
 Signed-off-by: Stephen Boyd sb...@codeaurora.org
 ---
  drivers/input/misc/pmic8xxx-pwrkey.c | 284 
 ++-
  1 file changed, 277 insertions(+), 7 deletions(-)
 

Reviewed-by: Bjorn Andersson bjorn.anders...@sonymobile.com

Regards,
Bjorn
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] Input: pmic8xxx-pwrkey - Support shutdown

2015-07-13 Thread Dmitry Torokhov
Hi Stephen,

On Mon, Jul 13, 2015 at 04:30:34PM -0700, Stephen Boyd wrote:
 On pm8xxx PMICs, shutdown and restart are signaled to the PMIC
 via a pin called PS_HOLD. When this pin goes low, the PMIC
 performs a configurable power sequence. Add a .shutdown hook so
 that we can properly configure this power sequence for shutdown
 or restart depending on the system state.

Just a bit of bikeshedding on my part...

 
 Cc: Bjorn Andersson bjorn.anders...@sonymobile.com
 Signed-off-by: Stephen Boyd sb...@codeaurora.org
 ---
  drivers/input/misc/pmic8xxx-pwrkey.c | 284 
 ++-
  1 file changed, 277 insertions(+), 7 deletions(-)
 
 diff --git a/drivers/input/misc/pmic8xxx-pwrkey.c 
 b/drivers/input/misc/pmic8xxx-pwrkey.c
 index c4ca20e63221..1d1ee5951b3a 100644
 --- a/drivers/input/misc/pmic8xxx-pwrkey.c
 +++ b/drivers/input/misc/pmic8xxx-pwrkey.c
 @@ -20,17 +20,72 @@
  #include linux/regmap.h
  #include linux/log2.h
  #include linux/of.h
 +#include linux/of_device.h
  
  #define PON_CNTL_1 0x1C
  #define PON_CNTL_PULL_UP BIT(7)
  #define PON_CNTL_TRIG_DELAY_MASK (0x7)
 +#define PON_CNTL_1_PULL_UP_EN0xe0
 +#define PON_CNTL_1_USB_PWR_EN0x10
 +#define PON_CNTL_1_WD_EN_RESET   0x08
 +
 +#define PM8058_SLEEP_CTRL0x02b
 +#define PM8921_SLEEP_CTRL0x10a
 +
 +#define SLEEP_CTRL_SMPL_EN_RESET 0x04
 +
 +/* Regulator master enable addresses */
 +#define REG_PM8058_VREG_EN_MSM   0x018
 +#define REG_PM8058_VREG_EN_GRP_5_4   0x1C8
 +
 +/* Regulator control registers for shutdown/reset */
 +#define PM8058_S0_CTRL   0x004
 +#define PM8058_S1_CTRL   0x005
 +#define PM8058_S3_CTRL   0x111
 +#define PM8058_L21_CTRL  0x120
 +#define PM8058_L22_CTRL  0x121
 +
 +#define PM8058_REGULATOR_ENABLE_MASK 0x80
 +#define PM8058_REGULATOR_ENABLE  0x80
 +#define PM8058_REGULATOR_DISABLE 0x00
 +#define PM8058_REGULATOR_PULL_DOWN_MASK  0x40
 +#define PM8058_REGULATOR_PULL_DOWN_EN0x40
 +
 +/* Buck CTRL register */
 +#define PM8058_SMPS_LEGACY_VREF_SEL  0x20
 +#define PM8058_SMPS_LEGACY_VPROG_MASK0x1F
 +#define PM8058_SMPS_ADVANCED_BAND_MASK   0xC0
 +#define PM8058_SMPS_ADVANCED_BAND_SHIFT  6
 +#define PM8058_SMPS_ADVANCED_VPROG_MASK  0x3F
 +
 +/* Buck TEST2 registers for shutdown/reset */
 +#define PM8058_S0_TEST2  0x084
 +#define PM8058_S1_TEST2  0x085
 +#define PM8058_S3_TEST2  0x11A
 +
 +#define PM8058_REGULATOR_BANK_WRITE  0x80
 +#define PM8058_REGULATOR_BANK_MASK   0x70
 +#define PM8058_REGULATOR_BANK_SHIFT  4
 +#define PM8058_REGULATOR_BANK_SEL(n) ((n)  PM8058_REGULATOR_BANK_SHIFT)
 +
 +/* Buck TEST2 register bank 1 */
 +#define PM8058_SMPS_LEGACY_VLOW_SEL  0x01
 +
 +/* Buck TEST2 register bank 7 */
 +#define PM8058_SMPS_ADVANCED_MODE_MASK   0x02
 +#define PM8058_SMPS_ADVANCED_MODE0x02
 +#define PM8058_SMPS_LEGACY_MODE  0x00
  
  /**
   * struct pmic8xxx_pwrkey - pmic8xxx pwrkey information
   * @key_press_irq: key press irq number
 + * @regmap: device regmap
 + * @shutdown_fn: shutdown configuration function
   */
  struct pmic8xxx_pwrkey {
   int key_press_irq;
 + struct regmap *regmap;
 + int (*shutdown_fn)(struct pmic8xxx_pwrkey *, bool);
  };
  
  static irqreturn_t pwrkey_press_irq(int irq, void *_pwr)
 @@ -76,6 +131,220 @@ static int __maybe_unused pmic8xxx_pwrkey_resume(struct 
 device *dev)
  static SIMPLE_DEV_PM_OPS(pm8xxx_pwr_key_pm_ops,
   pmic8xxx_pwrkey_suspend, pmic8xxx_pwrkey_resume);
  
 +static void pmic8xxx_pwrkey_shutdown(struct platform_device *pdev)
 +{
 + struct pmic8xxx_pwrkey *pwrkey = platform_get_drvdata(pdev);
 + int ret;
 + u8 mask, val;
 + bool reset = system_state == SYSTEM_RESTART;
 +
 + if (pwrkey-shutdown_fn) {
 + ret = pwrkey-shutdown_fn(pwrkey, reset);
 + if (ret)
 + return;

Can we call this variable error please?

 + }
 +
 + /*
 +  * Select action to perform (reset or shutdown) when PS_HOLD goes low.
 +  * Also ensure that KPD, CBL0, and CBL1 pull ups are enabled and that
 +  * USB charging is enabled.
 +  */
 + mask = PON_CNTL_1_PULL_UP_EN | PON_CNTL_1_USB_PWR_EN;
 + mask |= PON_CNTL_1_WD_EN_RESET;
 + val = mask;
 + if (!reset)
 + val = ~PON_CNTL_1_WD_EN_RESET;
 +
 + regmap_update_bits(pwrkey-regmap, PON_CNTL_1, mask, val);
 +}
 +
 +/*
 + * Set an SMPS regulator to be disabled in its CTRL register, but enabled
 + * in the master enable register.  Also set it's pull 

Re: [PATCH v2] Input: pmic8xxx-pwrkey - Support shutdown

2015-07-13 Thread Stephen Boyd

On 07/13/2015 05:25 PM, Dmitry Torokhov wrote:


@@ -76,6 +131,220 @@ static int __maybe_unused pmic8xxx_pwrkey_resume(struct 
device *dev)
  static SIMPLE_DEV_PM_OPS(pm8xxx_pwr_key_pm_ops,
pmic8xxx_pwrkey_suspend, pmic8xxx_pwrkey_resume);
  
+static void pmic8xxx_pwrkey_shutdown(struct platform_device *pdev)

+{
+   struct pmic8xxx_pwrkey *pwrkey = platform_get_drvdata(pdev);
+   int ret;
+   u8 mask, val;
+   bool reset = system_state == SYSTEM_RESTART;
+
+   if (pwrkey-shutdown_fn) {
+   ret = pwrkey-shutdown_fn(pwrkey, reset);
+   if (ret)
+   return;

Can we call this variable error please?


Ok.




+   }
+
+   /*
+* Select action to perform (reset or shutdown) when PS_HOLD goes low.
+* Also ensure that KPD, CBL0, and CBL1 pull ups are enabled and that
+* USB charging is enabled.
+*/
+   mask = PON_CNTL_1_PULL_UP_EN | PON_CNTL_1_USB_PWR_EN;
+   mask |= PON_CNTL_1_WD_EN_RESET;
+   val = mask;
+   if (!reset)
+   val = ~PON_CNTL_1_WD_EN_RESET;
+
+   regmap_update_bits(pwrkey-regmap, PON_CNTL_1, mask, val);
+}
+
+/*
+ * Set an SMPS regulator to be disabled in its CTRL register, but enabled
+ * in the master enable register.  Also set it's pull down enable bit.
+ * Take care to make sure that the output voltage doesn't change if switching
+ * from advanced mode to legacy mode.
+ */
+static int pm8058_disable_smps_locally_set_pull_down(struct regmap *regmap,
+   u16 ctrl_addr, u16 test2_addr, u16 master_enable_addr,
+   u8 master_enable_bit)
+{
+   int ret = 0;

Why does it need to be initialized? And error too please.


Doesn't need to be.




+
+   /* Enable LDO in master control register. */
+   ret = regmap_update_bits(regmap, master_enable_addr,
+   master_enable_bit, master_enable_bit);
+   if (ret)
+   return ret;
+
+   /* Disable LDO in CTRL register and set pull down */
+   return regmap_update_bits(regmap, ctrl_addr,
+   PM8058_REGULATOR_ENABLE_MASK | PM8058_REGULATOR_PULL_DOWN_MASK,
+   PM8058_REGULATOR_DISABLE | PM8058_REGULATOR_PULL_DOWN_EN);
+}
+
+static int pm8058_pwrkey_shutdown(struct pmic8xxx_pwrkey *pwrkey, bool reset)
+{
+   int ret;

And here.


Ok.




+   struct regmap *regmap = pwrkey-regmap;
+   u8 mask, val;
+
+   /* When shutting down, enable active pulldowns on important rails. */
+   if (!reset) {
+   /* Disable SMPS's 0,1,3 locally and set pulldown enable bits. */
+   pm8058_disable_smps_locally_set_pull_down(regmap,
+   PM8058_S0_CTRL, PM8058_S0_TEST2,
+   REG_PM8058_VREG_EN_MSM, BIT(7));
+   pm8058_disable_smps_locally_set_pull_down(regmap,
+   PM8058_S1_CTRL, PM8058_S1_TEST2,
+   REG_PM8058_VREG_EN_MSM, BIT(6));
+   pm8058_disable_smps_locally_set_pull_down(regmap,
+   PM8058_S3_CTRL, PM8058_S3_TEST2,
+   REG_PM8058_VREG_EN_GRP_5_4, BIT(7) | BIT(4));
+   /* Disable LDO 21 locally and set pulldown enable bit. */
+   pm8058_disable_ldo_locally_set_pull_down(regmap,
+   PM8058_L21_CTRL, REG_PM8058_VREG_EN_GRP_5_4,
+   BIT(1));
+   }
+
+   /*
+* Fix-up: Set regulator LDO22 to 1.225 V in high power mode. Leave its
+* pull-down state intact. This ensures a safe shutdown.
+*/
+   ret = regmap_update_bits(regmap, PM8058_L22_CTRL, 0xbf, 0x93);
+   if (ret)
+   return ret;
+
+   /* Enable SMPL if resetting is desired */
+   mask = SLEEP_CTRL_SMPL_EN_RESET;
+   val = 0;
+   if (reset)
+   val = mask;
+   return regmap_update_bits(regmap, PM8058_SLEEP_CTRL, mask, val);
+

Stray empty line.


Ok.




+}
+
+static int pm8921_pwrkey_shutdown(struct pmic8xxx_pwrkey *pwrkey, bool reset)
+{
+   struct regmap *regmap = pwrkey-regmap;
+   u8 mask = SLEEP_CTRL_SMPL_EN_RESET;
+   u8 val = 0;
+
+   /* Enable SMPL if resetting is desired */
+   if (reset)
+   val = mask;
+   return regmap_update_bits(regmap, PM8921_SLEEP_CTRL, mask, val);
+}
+
+static const struct of_device_id pm8xxx_pwr_key_id_table[] = {
+   { .compatible = qcom,pm8058-pwrkey, .data = pm8058_pwrkey_shutdown },
+   { .compatible = qcom,pm8921-pwrkey, .data = pm8921_pwrkey_shutdown },
+   { }
+};
+MODULE_DEVICE_TABLE(of, pm8xxx_pwr_key_id_table);
+

Can we please keep IDs and device table where it was, close to the
driver definition?


That would require forward declaring the array here. Is that desired? I 
moved it so that I could use it in the probe function.


--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the 

Re: [PATCH v2] Input: pmic8xxx-pwrkey - Support shutdown

2015-07-13 Thread Stephen Boyd

On 07/13/2015 05:46 PM, Dmitry Torokhov wrote:

On Mon, Jul 13, 2015 at 05:34:57PM -0700, Stephen Boyd wrote:

On 07/13/2015 05:25 PM, Dmitry Torokhov wrote:

+
+static const struct of_device_id pm8xxx_pwr_key_id_table[] = {
+   { .compatible = qcom,pm8058-pwrkey, .data = pm8058_pwrkey_shutdown },
+   { .compatible = qcom,pm8921-pwrkey, .data = pm8921_pwrkey_shutdown },
+   { }
+};
+MODULE_DEVICE_TABLE(of, pm8xxx_pwr_key_id_table);
+

Can we please keep IDs and device table where it was, close to the
driver definition?

That would require forward declaring the array here. Is that
desired? I moved it so that I could use it in the probe function.

Ah, yes, OF data is not passed into probe() :(... But we can always do:

match = of_match_device(pdev-dev.driver-of_match_table, pdev-dev);

Maybe we could have a helper for this like:

const struct of_device_id *of_get_current_match(struct device *dev)
{
return dev-drv ?
of_match_device(dev-driver-of_match_table, dev) :
NULL;
}


Nice. I'll update and Cc the OF maintainers.

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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


Re: [PATCH v2] Input: pmic8xxx-pwrkey - Support shutdown

2015-07-13 Thread Stephen Boyd

On 07/13/2015 05:58 PM, Stephen Boyd wrote:

On 07/13/2015 05:46 PM, Dmitry Torokhov wrote:

On Mon, Jul 13, 2015 at 05:34:57PM -0700, Stephen Boyd wrote:

On 07/13/2015 05:25 PM, Dmitry Torokhov wrote:

+
+static const struct of_device_id pm8xxx_pwr_key_id_table[] = {
+{ .compatible = qcom,pm8058-pwrkey, .data = 
pm8058_pwrkey_shutdown },
+{ .compatible = qcom,pm8921-pwrkey, .data = 
pm8921_pwrkey_shutdown },

+{ }
+};
+MODULE_DEVICE_TABLE(of, pm8xxx_pwr_key_id_table);
+

Can we please keep IDs and device table where it was, close to the
driver definition?

That would require forward declaring the array here. Is that
desired? I moved it so that I could use it in the probe function.

Ah, yes, OF data is not passed into probe() :(... But we can always do:

match = of_match_device(pdev-dev.driver-of_match_table, 
pdev-dev);


Maybe we could have a helper for this like:

const struct of_device_id *of_get_current_match(struct device *dev)
{
return dev-drv ?
of_match_device(dev-driver-of_match_table, dev) :
NULL;
}


Nice. I'll update and Cc the OF maintainers.



Actually, this already exists I just didn't know. See 
of_device_get_match_data().


--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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