Hi Manaf, Typo: fix zeorC in subject line. Please rebase this patch[1] on top of my patch merging tsens-common.c and tsens.c. [1] https://lore.kernel.org/linux-arm-msm/e30e2ba6fa5c007983afd4d7d4e0311c0b57917a.1588183879.git.amit.kucheria@xxxxxxxxxx/ On Tue, May 5, 2020 at 4:42 PM Manaf Meethalavalappu Pallikunhi <manafm@xxxxxxxxxxxxxx> wrote: > > TSENS IP v2.6+ adds 0C interrupt support. It triggers set > interrupt when aggregated minimum temperature of all TSENS falls > below 0C preset threshold and triggers reset interrupt when aggregate > minimum temperature of all TSENS crosses above reset threshold. > Add support for this interrupt in the driver. > > It adds another sensor to the of-thermal along with all individual > TSENS. It enables to add any mitigation for 0C interrupt. > > Signed-off-by: Manaf Meethalavalappu Pallikunhi <manafm@xxxxxxxxxxxxxx> > --- > drivers/thermal/qcom/tsens-common.c | 72 ++++++++++++++++++++++++++++- > drivers/thermal/qcom/tsens-v2.c | 7 +++ > drivers/thermal/qcom/tsens.c | 51 ++++++++++++++++++-- > drivers/thermal/qcom/tsens.h | 11 +++++ > 4 files changed, 135 insertions(+), 6 deletions(-) > > diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c > index 172545366636..44e7edeb9a90 100644 > --- a/drivers/thermal/qcom/tsens-common.c > +++ b/drivers/thermal/qcom/tsens-common.c > @@ -198,7 +198,8 @@ static void tsens_set_interrupt_v1(struct tsens_priv *priv, u32 hw_id, > index = LOW_INT_CLEAR_0 + hw_id; > break; > case CRITICAL: > - /* No critical interrupts before v2 */ > + case ZEROC: > + /* No critical and 0c interrupts before v2 */ > return; > } > regmap_field_write(priv->rf[index], enable ? 0 : 1); > @@ -229,6 +230,9 @@ static void tsens_set_interrupt_v2(struct tsens_priv *priv, u32 hw_id, > index_mask = CRIT_INT_MASK_0 + hw_id; > index_clear = CRIT_INT_CLEAR_0 + hw_id; > break; > + case ZEROC: > + /* Nothing to handle for 0c interrupt */ > + return; > } > > if (enable) { > @@ -360,6 +364,34 @@ static inline u32 masked_irq(u32 hw_id, u32 mask, enum tsens_ver ver) > return 0; > } > > +/** > + * tsens_0c_irq_thread - Threaded interrupt handler for 0c interrupt Let's use zeroc instead of 0c in the function and variable names and comments everywhere. Easier to grep and better consistency too. > + * @irq: irq number > + * @data: tsens controller private data > + * > + * Whenever interrupt triggers notify thermal framework using > + * thermal_zone_device_update() to update cold temperature mitigation. How is this mitigation updated? > + * > + * Return: IRQ_HANDLED > + */ > +irqreturn_t tsens_0c_irq_thread(int irq, void *data) > +{ > + struct tsens_priv *priv = data; > + struct tsens_sensor *s = &priv->sensor[priv->num_sensors]; > + int temp, ret; > + > + ret = regmap_field_read(priv->rf[TSENS_0C_STATUS], &temp); > + if (ret) > + return ret; > + > + dev_dbg(priv->dev, "[%u] %s: 0c interrupt is %s\n", > + s->hw_id, __func__, temp ? "triggered" : "cleared"); So triggered is printed for non-zero (including negative) values? > + > + thermal_zone_device_update(s->tzd, THERMAL_EVENT_UNSPECIFIED); > + > + return IRQ_HANDLED; > +} > + > /** > * tsens_critical_irq_thread() - Threaded handler for critical interrupts > * @irq: irq number > @@ -566,6 +598,20 @@ void tsens_disable_irq(struct tsens_priv *priv) > regmap_field_write(priv->rf[INT_EN], 0); > } > > +int tsens_get_0c_int_status(const struct tsens_sensor *s, int *temp) > +{ > + struct tsens_priv *priv = s->priv; > + int last_temp = 0, ret; > + > + ret = regmap_field_read(priv->rf[TSENS_0C_STATUS], &last_temp); > + if (ret) > + return ret; > + > + *temp = last_temp; > + > + return 0; > +} > + > int get_temp_tsens_valid(const struct tsens_sensor *s, int *temp) > { > struct tsens_priv *priv = s->priv; > @@ -833,6 +879,30 @@ int __init init_common(struct tsens_priv *priv) > regmap_field_write(priv->rf[CC_MON_MASK], 1); > } > > + if (tsens_version(priv) > VER_1_X && ver_minor > 5) { > + /* 0C interrupt is present only on v2.6+ */ > + priv->rf[TSENS_0C_INT_EN] = devm_regmap_field_alloc(dev, > + priv->srot_map, > + priv->fields[TSENS_0C_INT_EN]); > + if (IS_ERR(priv->rf[TSENS_0C_INT_EN])) { > + ret = PTR_ERR(priv->rf[TSENS_0C_INT_EN]); > + goto err_put_device; > + } > + > + /* Check whether 0C interrupt is enabled or not */ > + regmap_field_read(priv->rf[TSENS_0C_INT_EN], &enabled); > + if (enabled) { > + priv->feat->zero_c_int = 1; This should be done at the beginning of the block where you check our version is > 2.6 since the flag only says whether the feature is present. > + priv->rf[TSENS_0C_STATUS] = devm_regmap_field_alloc(dev, > + priv->tm_map, > + priv->fields[TSENS_0C_STATUS]); > + if (IS_ERR(priv->rf[TSENS_0C_STATUS])) { > + ret = PTR_ERR(priv->rf[TSENS_0C_STATUS]); > + goto err_put_device; > + } > + } > + } > + > spin_lock_init(&priv->ul_lock); > tsens_enable_irq(priv); > tsens_debug_init(op); > diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c > index b293ed32174b..ce80d82c7255 100644 > --- a/drivers/thermal/qcom/tsens-v2.c > +++ b/drivers/thermal/qcom/tsens-v2.c > @@ -11,6 +11,7 @@ > /* ----- SROT ------ */ > #define SROT_HW_VER_OFF 0x0000 > #define SROT_CTRL_OFF 0x0004 > +#define SROT_OC_CTRL_OFF 0x0018 > > /* ----- TM ------ */ > #define TM_INT_EN_OFF 0x0004 > @@ -23,6 +24,7 @@ > #define TM_Sn_UPPER_LOWER_THRESHOLD_OFF 0x0020 > #define TM_Sn_CRITICAL_THRESHOLD_OFF 0x0060 > #define TM_Sn_STATUS_OFF 0x00a0 > +#define TM_0C_INT_STATUS_OFF 0x00e0 > #define TM_TRDY_OFF 0x00e4 > #define TM_WDOG_LOG_OFF 0x013c > > @@ -45,6 +47,7 @@ static const struct reg_field tsens_v2_regfields[MAX_REGFIELDS] = { > /* CTRL_OFF */ > [TSENS_EN] = REG_FIELD(SROT_CTRL_OFF, 0, 0), > [TSENS_SW_RST] = REG_FIELD(SROT_CTRL_OFF, 1, 1), > + [TSENS_0C_INT_EN] = REG_FIELD(SROT_OC_CTRL_OFF, 0, 0), > > /* ----- TM ------ */ > /* INTERRUPT ENABLE */ > @@ -86,6 +89,9 @@ static const struct reg_field tsens_v2_regfields[MAX_REGFIELDS] = { > REG_FIELD_FOR_EACH_SENSOR16(CRITICAL_STATUS, TM_Sn_STATUS_OFF, 19, 19), > REG_FIELD_FOR_EACH_SENSOR16(MAX_STATUS, TM_Sn_STATUS_OFF, 20, 20), > > + /* 0C INETRRUPT STATUS */ Typo: Interrupt > + [TSENS_0C_STATUS] = REG_FIELD(TM_0C_INT_STATUS_OFF, 0, 0), > + > /* TRDY: 1=ready, 0=in progress */ > [TRDY] = REG_FIELD(TM_TRDY_OFF, 0, 0), > }; > @@ -93,6 +99,7 @@ static const struct reg_field tsens_v2_regfields[MAX_REGFIELDS] = { > static const struct tsens_ops ops_generic_v2 = { > .init = init_common, > .get_temp = get_temp_tsens_valid, > + .get_0c_status = tsens_get_0c_int_status, > }; > > struct tsens_plat_data data_tsens_v2 = { > diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c > index 2f77d235cf73..e60870c53383 100644 > --- a/drivers/thermal/qcom/tsens.c > +++ b/drivers/thermal/qcom/tsens.c > @@ -14,6 +14,17 @@ > #include <linux/thermal.h> > #include "tsens.h" > > +static int tsens_0c_get_temp(void *data, int *temp) > +{ > + struct tsens_sensor *s = data; > + struct tsens_priv *priv = s->priv; > + > + if (priv->ops->get_0c_status) > + return priv->ops->get_0c_status(s, temp); > + > + return -ENOTSUPP; > +} > + > static int tsens_get_temp(void *data, int *temp) > { > struct tsens_sensor *s = data; > @@ -85,6 +96,10 @@ static const struct thermal_zone_of_device_ops tsens_of_ops = { > .set_trips = tsens_set_trips, > }; > > +static const struct thermal_zone_of_device_ops tsens_0c_of_ops = { > + .get_temp = tsens_0c_get_temp, > +}; > + > static int tsens_register_irq(struct tsens_priv *priv, char *irqname, > irq_handler_t thread_fn) > { > @@ -142,6 +157,21 @@ static int tsens_register(struct tsens_priv *priv) > ret = tsens_register_irq(priv, "critical", > tsens_critical_irq_thread); > > + if (priv->feat->zero_c_int) { > + priv->sensor[priv->num_sensors].priv = priv; > + tzd = devm_thermal_zone_of_sensor_register(priv->dev, > + priv->sensor[priv->num_sensors].hw_id, > + &priv->sensor[priv->num_sensors], > + &tsens_0c_of_ops); > + if (IS_ERR(tzd)) { > + ret = 0; > + return ret; > + } > + > + priv->sensor[priv->num_sensors].tzd = tzd; Why can't this happen in the previous loop, but increase the loop to <= num_sensors? It is duplicated code. > + ret = tsens_register_irq(priv, "zeroc", tsens_0c_irq_thread); > + } > + > return ret; > } > > @@ -178,11 +208,22 @@ static int tsens_probe(struct platform_device *pdev) > return -EINVAL; > } > > - priv = devm_kzalloc(dev, > - struct_size(priv, sensor, num_sensors), > - GFP_KERNEL); > - if (!priv) > - return -ENOMEM; > + /* Check for 0c interrupt is enabled or not */ > + if (platform_get_irq_byname(pdev, "zeroc") > 0) { > + priv = devm_kzalloc(dev, > + struct_size(priv, sensor, num_sensors + 1), > + GFP_KERNEL); Instead of doing this, simply do the following, if (platform_get_irq_byname(pdev, "zeroc") > 0) { num_sensors++; The kzalloc will just work then, no? > + if (!priv) > + return -ENOMEM; > + /* Use Max sensor index as 0c sensor hw_id */ > + priv->sensor[num_sensors].hw_id = data->feat->max_sensors; > + } else { > + priv = devm_kzalloc(dev, > + struct_size(priv, sensor, num_sensors), > + GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + } > > priv->dev = dev; > priv->num_sensors = num_sensors; > diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h > index 502acf0e6828..5b53a0352b4d 100644 > --- a/drivers/thermal/qcom/tsens.h > +++ b/drivers/thermal/qcom/tsens.h > @@ -34,6 +34,7 @@ enum tsens_irq_type { > LOWER, > UPPER, > CRITICAL, > + ZEROC, > }; > > /** > @@ -64,6 +65,7 @@ struct tsens_sensor { > * @suspend: Function to suspend the tsens device > * @resume: Function to resume the tsens device > * @get_trend: Function to get the thermal/temp trend > + * @get_0c_status: Function to get the 0c interrupt status > */ > struct tsens_ops { > /* mandatory callbacks */ > @@ -76,6 +78,7 @@ struct tsens_ops { > int (*suspend)(struct tsens_priv *priv); > int (*resume)(struct tsens_priv *priv); > int (*get_trend)(struct tsens_sensor *s, enum thermal_trend *trend); > + int (*get_0c_status)(const struct tsens_sensor *s, int *temp); > }; > > #define REG_FIELD_FOR_EACH_SENSOR11(_name, _offset, _startbit, _stopbit) \ > @@ -161,6 +164,8 @@ enum regfield_ids { > TSENS_SW_RST, > SENSOR_EN, > CODE_OR_TEMP, > + /* 0C CTRL OFFSET */ > + TSENS_0C_INT_EN, > > /* ----- TM ------ */ > /* TRDY */ > @@ -485,6 +490,8 @@ enum regfield_ids { > MAX_STATUS_14, > MAX_STATUS_15, > > + TSENS_0C_STATUS, /* 0C INTERRUPT status */ > + > /* Keep last */ > MAX_REGFIELDS > }; > @@ -497,6 +504,7 @@ enum regfield_ids { > * @srot_split: does the IP neatly splits the register space into SROT and TM, > * with SROT only being available to secure boot firmware? > * @has_watchdog: does this IP support watchdog functionality? > + * @zero_c_int: does this IP support 0C interrupt ? > * @max_sensors: maximum sensors supported by this version of the IP > */ > struct tsens_features { > @@ -505,6 +513,7 @@ struct tsens_features { > unsigned int adc:1; > unsigned int srot_split:1; > unsigned int has_watchdog:1; > + unsigned int zero_c_int:1; zeroc_interrupt > unsigned int max_sensors; > }; > > @@ -580,11 +589,13 @@ void compute_intercept_slope(struct tsens_priv *priv, u32 *pt1, u32 *pt2, u32 mo > int init_common(struct tsens_priv *priv); > int get_temp_tsens_valid(const struct tsens_sensor *s, int *temp); > int get_temp_common(const struct tsens_sensor *s, int *temp); > +int tsens_get_0c_int_status(const struct tsens_sensor *s, int *temp); > int tsens_enable_irq(struct tsens_priv *priv); > void tsens_disable_irq(struct tsens_priv *priv); > int tsens_set_trips(void *_sensor, int low, int high); > irqreturn_t tsens_irq_thread(int irq, void *data); > irqreturn_t tsens_critical_irq_thread(int irq, void *data); > +irqreturn_t tsens_0c_irq_thread(int irq, void *data); > > /* TSENS target */ > extern struct tsens_plat_data data_8960; > -- > 2.26.2
