Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754473AbdHYCM7 (ORCPT ); Thu, 24 Aug 2017 22:12:59 -0400 Received: from mga04.intel.com ([192.55.52.120]:33217 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754216AbdHYCM5 (ORCPT ); Thu, 24 Aug 2017 22:12:57 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,423,1498546800"; d="scan'208";a="128031438" Message-ID: <1503627189.2774.3.camel@intel.com> Subject: Re: [PATCH V1] thermal: qcom-spmi-temp-alarm: add support for GEN2 PMIC peripherals From: Zhang Rui To: kgunda@codeaurora.org Cc: Eduardo Valentin , linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, iivanov@mm-sol.com, linux-arm-msm@vger.kernel.org, David Collins , linux-arm-msm-owner@vger.kernel.org, Geert Uytterhoeven , Jean Delvare , Lukasz Majewski , Darren Hart , Heiko Stuebner , Peter Feuerer , bjorn.andersson@linaro.org, sboyd@codeaurora.org Date: Fri, 25 Aug 2017 10:13:09 +0800 In-Reply-To: <5d3f612dca4181b2038e442cfba5594e@codeaurora.org> References: <1499947772-19199-1-git-send-email-kgunda@codeaurora.org> <1502179943.4296.12.camel@intel.com> <5d3f612dca4181b2038e442cfba5594e@codeaurora.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.5.2-0ubuntu3 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9425 Lines: 286 On Thu, 2017-08-17 at 13:12 +0530, kgunda@codeaurora.org wrote: > On 2017-08-16 17:53, kgunda@codeaurora.org wrote: > > > > On 2017-08-08 13:42, Zhang Rui wrote: > > > > > > On Thu, 2017-07-13 at 17:39 +0530, Kiran Gunda wrote: > > > > > > > > From: David Collins > > > > > > > > Add support for the TEMP_ALARM GEN2 PMIC peripheral > > > > subtype.  The > > > > GEN2 subtype defines an over temperature state with hysteresis > > > > instead of stage in the status register.  There are two GEN2 > > > > states corresponding to stages 1 and 2. > > > > > > > > Signed-off-by: David Collins > > > > Signed-off-by: Kiran Gunda > > > Ivan, > > > > > > can you please review this patch and let me know your opinion? > > > > > > thanks, > > > rui > > Ivan, > > Could you please review this patch ? > > > > Thanks, > > Kiran > Looks like Ivan is no more reviewing the patches for qcom. > Adding Bjorn and Stephen Boyd for the review. > Given this is a platform specific change, I will queue it for next merge window, and let's see if there is any problem reported. thanks, rui > Thanks, > Kiran > > > > > > > > > > > > > --- > > > >  drivers/thermal/qcom-spmi-temp-alarm.c | 92 > > > > ++++++++++++++++++++++++++-------- > > > >  1 file changed, 71 insertions(+), 21 deletions(-) > > > > > > > > diff --git a/drivers/thermal/qcom-spmi-temp-alarm.c > > > > b/drivers/thermal/qcom-spmi-temp-alarm.c > > > > index f502419..a5e17ba 100644 > > > > --- a/drivers/thermal/qcom-spmi-temp-alarm.c > > > > +++ b/drivers/thermal/qcom-spmi-temp-alarm.c > > > > @@ -1,5 +1,5 @@ > > > >  /* > > > > - * Copyright (c) 2011-2015, The Linux Foundation. All rights > > > > reserved. > > > > + * Copyright (c) 2011-2015, 2017, The Linux Foundation. All > > > > rights > > > > reserved. > > > >   * > > > >   * This program is free software; you can redistribute it > > > > and/or > > > > modify > > > >   * it under the terms of the GNU General Public License > > > > version 2 > > > > and > > > > @@ -11,6 +11,7 @@ > > > >   * GNU General Public License for more details. > > > >   */ > > > >   > > > > +#include > > > >  #include > > > >  #include > > > >  #include > > > > @@ -29,13 +30,17 @@ > > > >  #define QPNP_TM_REG_ALARM_CTRL 0x46 > > > >   > > > >  #define QPNP_TM_TYPE 0x09 > > > > -#define QPNP_TM_SUBTYPE 0x08 > > > > +#define QPNP_TM_SUBTYPE_GEN1 0x08 > > > > +#define QPNP_TM_SUBTYPE_GEN2 0x09 > > > >   > > > > -#define STATUS_STAGE_MASK 0x03 > > > > +#define STATUS_GEN1_STAGE_MASK GENMASK(1, 0) > > > > +#define STATUS_GEN2_STATE_MASK GENMASK(6, 4) > > > > +#define STATUS_GEN2_STATE_SHIFT 4 > > > >   > > > > -#define SHUTDOWN_CTRL1_THRESHOLD_MASK 0x03 > > > > +#define SHUTDOWN_CTRL1_OVERRIDE_MASK GENMASK(7, 6) > > > > +#define SHUTDOWN_CTRL1_THRESHOLD_MASK GENMASK(1, 0) > > > >   > > > > -#define ALARM_CTRL_FORCE_ENABLE 0x80 > > > > +#define ALARM_CTRL_FORCE_ENABLE BIT(7) > > > >   > > > >  /* > > > >   * Trip point values based on threshold control > > > > @@ -58,6 +63,7 @@ > > > >  struct qpnp_tm_chip { > > > >   struct regmap *map; > > > >   struct thermal_zone_device *tz_dev; > > > > + unsigned int subtype; > > > >   long temp; > > > >   unsigned int thresh; > > > >   unsigned int stage; > > > > @@ -66,6 +72,9 @@ struct qpnp_tm_chip { > > > >   struct iio_channel *adc; > > > >  }; > > > >   > > > > +/* This array maps from GEN2 alarm state to GEN1 alarm stage > > > > */ > > > > +static const unsigned int alarm_state_map[8] = {0, 1, 1, 2, 2, > > > > 3, 3, > > > > 3}; > > > > + > > > >  static int qpnp_tm_read(struct qpnp_tm_chip *chip, u16 addr, > > > > u8 > > > > *data) > > > >  { > > > >   unsigned int val; > > > > @@ -84,30 +93,59 @@ static int qpnp_tm_write(struct > > > > qpnp_tm_chip > > > > *chip, u16 addr, u8 data) > > > >   return regmap_write(chip->map, chip->base + addr, > > > > data); > > > >  } > > > >   > > > > +/** > > > > + * qpnp_tm_get_temp_stage() - return over-temperature stage > > > > + * @chip: Pointer to the qpnp_tm chip > > > > + * > > > > + * Return: stage (GEN1) or state (GEN2) on success, or errno > > > > on > > > > failure. > > > > + */ > > > > +static int qpnp_tm_get_temp_stage(struct qpnp_tm_chip *chip) > > > > +{ > > > > + int ret; > > > > + u8 reg = 0; > > > > + > > > > + ret = qpnp_tm_read(chip, QPNP_TM_REG_STATUS, ®); > > > > + if (ret < 0) > > > > + return ret; > > > > + > > > > + if (chip->subtype == QPNP_TM_SUBTYPE_GEN1) > > > > + ret = reg & STATUS_GEN1_STAGE_MASK; > > > > + else > > > > + ret = (reg & STATUS_GEN2_STATE_MASK) >> > > > > STATUS_GEN2_STATE_SHIFT; > > > > + > > > > + return ret; > > > > +} > > > > + > > > >  /* > > > >   * This function updates the internal temp value based on the > > > >   * current thermal stage and threshold as well as the previous > > > > stage > > > >   */ > > > >  static int qpnp_tm_update_temp_no_adc(struct qpnp_tm_chip > > > > *chip) > > > >  { > > > > - unsigned int stage; > > > > + unsigned int stage, stage_new, stage_old; > > > >   int ret; > > > > - u8 reg = 0; > > > >   > > > > - ret = qpnp_tm_read(chip, QPNP_TM_REG_STATUS, ®); > > > > + ret = qpnp_tm_get_temp_stage(chip); > > > >   if (ret < 0) > > > >   return ret; > > > > + stage = ret; > > > >   > > > > - stage = reg & STATUS_STAGE_MASK; > > > > + if (chip->subtype == QPNP_TM_SUBTYPE_GEN1) { > > > > + stage_new = stage; > > > > + stage_old = chip->stage; > > > > + } else { > > > > + stage_new = alarm_state_map[stage]; > > > > + stage_old = alarm_state_map[chip->stage]; > > > > + } > > > >   > > > > - if (stage > chip->stage) { > > > > + if (stage_new > stage_old) { > > > >   /* increasing stage, use lower bound */ > > > > - chip->temp = (stage - 1) * TEMP_STAGE_STEP + > > > > + chip->temp = (stage_new - 1) * TEMP_STAGE_STEP > > > > + > > > >        chip->thresh * TEMP_THRESH_STEP + > > > >        TEMP_STAGE_HYSTERESIS + > > > > TEMP_THRESH_MIN; > > > > - } else if (stage < chip->stage) { > > > > + } else if (stage_new < stage_old) { > > > >   /* decreasing stage, use upper bound */ > > > > - chip->temp = stage * TEMP_STAGE_STEP + > > > > + chip->temp = stage_new * TEMP_STAGE_STEP + > > > >        chip->thresh * TEMP_THRESH_STEP - > > > >        TEMP_STAGE_HYSTERESIS + > > > > TEMP_THRESH_MIN; > > > >   } > > > > @@ -162,28 +200,37 @@ static irqreturn_t qpnp_tm_isr(int irq, > > > > void > > > > *data) > > > >   */ > > > >  static int qpnp_tm_init(struct qpnp_tm_chip *chip) > > > >  { > > > > + unsigned int stage; > > > >   int ret; > > > > - u8 reg; > > > > + u8 reg = 0; > > > >   > > > > - chip->thresh = THRESH_MIN; > > > > + ret = qpnp_tm_read(chip, QPNP_TM_REG_SHUTDOWN_CTRL1, > > > > ®); > > > > + if (ret < 0) > > > > + return ret; > > > > + > > > > + chip->thresh = reg & SHUTDOWN_CTRL1_THRESHOLD_MASK; > > > >   chip->temp = DEFAULT_TEMP; > > > >   > > > > - ret = qpnp_tm_read(chip, QPNP_TM_REG_STATUS, ®); > > > > + ret = qpnp_tm_get_temp_stage(chip); > > > >   if (ret < 0) > > > >   return ret; > > > > + chip->stage = ret; > > > >   > > > > - chip->stage = reg & STATUS_STAGE_MASK; > > > > + stage = chip->subtype == QPNP_TM_SUBTYPE_GEN1 > > > > + ? chip->stage : alarm_state_map[chip->stage]; > > > >   > > > > - if (chip->stage) > > > > + if (stage) > > > >   chip->temp = chip->thresh * TEMP_THRESH_STEP + > > > > -      (chip->stage - 1) * > > > > TEMP_STAGE_STEP + > > > > +      (stage - 1) * TEMP_STAGE_STEP + > > > >        TEMP_THRESH_MIN; > > > >   > > > >   /* > > > >    * Set threshold and disable software override of > > > > stage 2 > > > > and 3 > > > >    * shutdowns. > > > >    */ > > > > - reg = chip->thresh & SHUTDOWN_CTRL1_THRESHOLD_MASK; > > > > + chip->thresh = THRESH_MIN; > > > > + reg &= ~(SHUTDOWN_CTRL1_OVERRIDE_MASK | > > > > SHUTDOWN_CTRL1_THRESHOLD_MASK); > > > > + reg |= chip->thresh & SHUTDOWN_CTRL1_THRESHOLD_MASK; > > > >   ret = qpnp_tm_write(chip, QPNP_TM_REG_SHUTDOWN_CTRL1, > > > > reg); > > > >   if (ret < 0) > > > >   return ret; > > > > @@ -242,13 +289,16 @@ static int qpnp_tm_probe(struct > > > > platform_device > > > > *pdev) > > > >   goto fail; > > > >   } > > > >   > > > > - if (type != QPNP_TM_TYPE || subtype != > > > > QPNP_TM_SUBTYPE) { > > > > + if (type != QPNP_TM_TYPE || (subtype != > > > > QPNP_TM_SUBTYPE_GEN1 > > > > +      && subtype != > > > > QPNP_TM_SUBTYPE_GEN2)) { > > > >   dev_err(&pdev->dev, "invalid type 0x%02x or > > > > subtype > > > > 0x%02x\n", > > > >   type, subtype); > > > >   ret = -ENODEV; > > > >   goto fail; > > > >   } > > > >   > > > > + chip->subtype = subtype; > > > > + > > > >   ret = qpnp_tm_init(chip); > > > >   if (ret < 0) { > > > >   dev_err(&pdev->dev, "init failed\n"); > > > -- > > > To unsubscribe from this list: send the line "unsubscribe  > > > linux-arm-msm" in > > > the body of a message to majordomo@vger.kernel.org > > > More majordomo info at  http://vger.kernel.org/majordomo-info.htm > > > l > > -- > > To unsubscribe from this list: send the line "unsubscribe  > > linux-arm-msm" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at  http://vger.kernel.org/majordomo-info.html