[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1503627189.2774.3.camel@intel.com>
Date: Fri, 25 Aug 2017 10:13:09 +0800
From: Zhang Rui <rui.zhang@...el.com>
To: kgunda@...eaurora.org
Cc: Eduardo Valentin <edubezval@...il.com>, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org, iivanov@...sol.com,
linux-arm-msm@...r.kernel.org,
David Collins <collinsd@...eaurora.org>,
linux-arm-msm-owner@...r.kernel.org,
Geert Uytterhoeven <geert+renesas@...der.be>,
Jean Delvare <jdelvare@...e.de>,
Lukasz Majewski <l.majewski@...sung.com>,
Darren Hart <dvhart@...ux.intel.com>,
Heiko Stuebner <heiko@...ech.de>,
Peter Feuerer <peter@...e.net>, bjorn.andersson@...aro.org,
sboyd@...eaurora.org
Subject: Re: [PATCH V1] thermal: qcom-spmi-temp-alarm: add support for GEN2
PMIC peripherals
On Thu, 2017-08-17 at 13:12 +0530, kgunda@...eaurora.org wrote:
> On 2017-08-16 17:53, kgunda@...eaurora.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 <collinsd@...eaurora.org>
> > > >
> > > > 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 <collinsd@...eaurora.org>
> > > > Signed-off-by: Kiran Gunda <kgunda@...eaurora.org>
> > > 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 <linux/bitops.h>
> > > > #include <linux/delay.h>
> > > > #include <linux/err.h>
> > > > #include <linux/iio/consumer.h>
> > > > @@ -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@...r.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@...r.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists