On Wed, Jul 22, 2020 at 12:52 AM <ansuelsmth@xxxxxxxxx> wrote: > > > > > -----Messaggio originale----- > > Da: Amit Kucheria <amit.kucheria@xxxxxxxxxx> > > Inviato: lunedì 20 luglio 2020 11:41 > > A: Ansuel Smith <ansuelsmth@xxxxxxxxx> > > Cc: Rob Herring <robh+dt@xxxxxxxxxx>; Andy Gross <agross@xxxxxxxxxx>; > > Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>; Zhang Rui > > <rui.zhang@xxxxxxxxx>; Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>; > > Michael Turquette <mturquette@xxxxxxxxxxxx>; Stephen Boyd > > <sboyd@xxxxxxxxxx>; Linux PM list <linux-pm@xxxxxxxxxxxxxxx>; linux-arm- > > msm <linux-arm-msm@xxxxxxxxxxxxxxx>; DTML > > <devicetree@xxxxxxxxxxxxxxx>; Linux Kernel Mailing List <linux- > > kernel@xxxxxxxxxxxxxxx>; linux-clk <linux-clk@xxxxxxxxxxxxxxx> > > Oggetto: Re: [PATCH v4 5/7] drivers: thermal: tsens: add interrupt support > > for 9860 driver > > > > Hi Ansuel, > > > > Thanks for this patch. > > > > On Thu, Jul 16, 2020 at 7:58 AM Ansuel Smith <ansuelsmth@xxxxxxxxx> > > wrote: > > > > > > Add interrupt support for 9860 tsens driver used to set thermal trip > > > point for the system. > > > > typo: 8960 > > > > You've used the names 8960 and ipq8064 interchangeably throughout the > > series. AFAICT, msm8960, ipq8064 and apq8064 use the same IP version > > of tsens. Please use 8960 in all patches, descriptions and dt-binding. > > to reflect the filename for the driver. > > Then add ipq8064 and apq8064 in a comment in the driver like here to > > show that the driver also supports these other SoCs: > > https://elixir.bootlin.com/linux/v5.8- > > rc4/source/drivers/thermal/qcom/tsens-v0_1.c#L328 > > > > You can also add a new compatible string for ipq8064 as a separate > > patch at the end of the series. > > > > > Signed-off-by: Ansuel Smith <ansuelsmth@xxxxxxxxx> > > > --- > > > drivers/thermal/qcom/tsens-8960.c | 197 > > +++++++++++++++++++++++++++--- > > > drivers/thermal/qcom/tsens.h | 3 + > > > 2 files changed, 186 insertions(+), 14 deletions(-) > > > > > > diff --git a/drivers/thermal/qcom/tsens-8960.c > > b/drivers/thermal/qcom/tsens-8960.c > > > index 45788eb3c666..20d0bfb10f1f 100644 > > > --- a/drivers/thermal/qcom/tsens-8960.c > > > +++ b/drivers/thermal/qcom/tsens-8960.c > > > @@ -8,6 +8,7 @@ > > > #include <linux/bitops.h> > > > #include <linux/regmap.h> > > > #include <linux/mfd/syscon.h> > > > +#include <linux/interrupt.h> > > > #include <linux/thermal.h> > > > #include "tsens.h" > > > > > > @@ -27,7 +28,6 @@ > > > /* CNTL_ADDR bitmasks */ > > > #define EN BIT(0) > > > #define SW_RST BIT(1) > > > -#define SENSOR0_EN BIT(3) > > > #define SLP_CLK_ENA BIT(26) > > > #define SLP_CLK_ENA_8660 BIT(24) > > > #define MEASURE_PERIOD 1 > > > @@ -41,14 +41,26 @@ > > > > > > #define THRESHOLD_ADDR 0x3624 > > > /* THRESHOLD_ADDR bitmasks */ > > > +#define THRESHOLD_MAX_CODE 0x20000 > > > +#define THRESHOLD_MIN_CODE 0 > > > #define THRESHOLD_MAX_LIMIT_SHIFT 24 > > > #define THRESHOLD_MIN_LIMIT_SHIFT 16 > > > #define THRESHOLD_UPPER_LIMIT_SHIFT 8 > > > #define THRESHOLD_LOWER_LIMIT_SHIFT 0 > > > +#define THRESHOLD_MAX_LIMIT_MASK (THRESHOLD_MAX_CODE > > << \ > > > + THRESHOLD_MAX_LIMIT_SHIFT) > > > +#define THRESHOLD_MIN_LIMIT_MASK (THRESHOLD_MAX_CODE << > > \ > > > + THRESHOLD_MIN_LIMIT_SHIFT) > > > +#define THRESHOLD_UPPER_LIMIT_MASK (THRESHOLD_MAX_CODE > > << \ > > > + THRESHOLD_UPPER_LIMIT_SHIFT) > > > +#define THRESHOLD_LOWER_LIMIT_MASK (THRESHOLD_MAX_CODE > > << \ > > > + THRESHOLD_LOWER_LIMIT_SHIFT) > > > > > > /* Initial temperature threshold values */ > > > -#define LOWER_LIMIT_TH 0x50 > > > -#define UPPER_LIMIT_TH 0xdf > > > +#define LOWER_LIMIT_TH_8960 0x50 > > > +#define UPPER_LIMIT_TH_8960 0xdf > > > +#define LOWER_LIMIT_TH_8064 0x9d /* 95C */ > > > +#define UPPER_LIMIT_TH_8064 0xa6 /* 105C */ > > > #define MIN_LIMIT_TH 0x0 > > > #define MAX_LIMIT_TH 0xff > > > > > > @@ -57,6 +69,170 @@ > > > #define TRDY_MASK BIT(7) > > > #define TIMEOUT_US 100 > > > > > > +#define TSENS_EN BIT(0) > > > +#define TSENS_SW_RST BIT(1) > > > +#define TSENS_ADC_CLK_SEL BIT(2) > > > +#define SENSOR0_EN BIT(3) > > > +#define SENSOR1_EN BIT(4) > > > +#define SENSOR2_EN BIT(5) > > > +#define SENSOR3_EN BIT(6) > > > +#define SENSOR4_EN BIT(7) > > > +#define SENSORS_EN (SENSOR0_EN | SENSOR1_EN | \ > > > + SENSOR2_EN | SENSOR3_EN | SENSOR4_EN) > > > +#define TSENS_8064_SENSOR5_EN BIT(8) > > > +#define TSENS_8064_SENSOR6_EN BIT(9) > > > +#define TSENS_8064_SENSOR7_EN BIT(10) > > > +#define TSENS_8064_SENSOR8_EN BIT(11) > > > +#define TSENS_8064_SENSOR9_EN BIT(12) > > > +#define TSENS_8064_SENSOR10_EN BIT(13) > > > +#define TSENS_8064_SENSORS_EN (SENSORS_EN | \ > > > + TSENS_8064_SENSOR5_EN | \ > > > + TSENS_8064_SENSOR6_EN | \ > > > + TSENS_8064_SENSOR7_EN | \ > > > + TSENS_8064_SENSOR8_EN | \ > > > + TSENS_8064_SENSOR9_EN | \ > > > + TSENS_8064_SENSOR10_EN) > > > + > > > +u32 tsens_8960_slope[] = { > > > + 1176, 1176, 1154, 1176, > > > + 1111, 1132, 1132, 1199, > > > + 1132, 1199, 1132 > > > + }; > > > + > > > +/* Temperature on y axis and ADC-code on x-axis */ > > > +static inline int code_to_mdegC(u32 adc_code, const struct > > tsens_sensor *s) > > > +{ > > > + int slope, offset; > > > + > > > + slope = thermal_zone_get_slope(s->tzd); > > > + offset = CAL_MDEGC - slope * s->offset; > > > + > > > + return adc_code * slope + offset; > > > +} > > > + > > > +static void notify_uspace_tsens_fn(struct work_struct *work) > > > +{ > > > + struct tsens_sensor *s = container_of(work, struct tsens_sensor, > > > + notify_work); > > > + > > > + sysfs_notify(&s->tzd->device.kobj, NULL, "type"); > > > +} > > > + > > > +static void tsens_scheduler_fn(struct work_struct *work) > > > +{ > > > + struct tsens_priv *priv = > > > + container_of(work, struct tsens_priv, tsens_work); > > > + unsigned int threshold, threshold_low, code, reg, sensor; > > > + unsigned long mask; > > > + bool upper_th_x, lower_th_x; > > > + int ret; > > > + > > > + ret = regmap_read(priv->tm_map, STATUS_CNTL_ADDR_8064, > > ®); > > > + if (ret) > > > + return; > > > + reg = reg | LOWER_STATUS_CLR | UPPER_STATUS_CLR; > > > + ret = regmap_write(priv->tm_map, STATUS_CNTL_ADDR_8064, reg); > > > + if (ret) > > > + return; > > > + > > > + mask = ~(LOWER_STATUS_CLR | UPPER_STATUS_CLR); > > > + ret = regmap_read(priv->tm_map, THRESHOLD_ADDR, &threshold); > > > + if (ret) > > > + return; > > > + threshold_low = (threshold & THRESHOLD_LOWER_LIMIT_MASK) >> > > > + THRESHOLD_LOWER_LIMIT_SHIFT; > > > + threshold = (threshold & THRESHOLD_UPPER_LIMIT_MASK) >> > > > + THRESHOLD_UPPER_LIMIT_SHIFT; > > > + > > > + ret = regmap_read(priv->tm_map, STATUS_CNTL_ADDR_8064, > > ®); > > > + if (ret) > > > + return; > > > + > > > + ret = regmap_read(priv->tm_map, CNTL_ADDR, &sensor); > > > + if (ret) > > > + return; > > > + sensor &= (uint32_t)TSENS_8064_SENSORS_EN; > > > + sensor >>= SENSOR0_SHIFT; > > > + > > > + /* Constraint: There is only 1 interrupt control register for all > > > + * 11 temperature sensor. So monitoring more than 1 sensor based > > > + * on interrupts will yield inconsistent result. To overcome this > > > + * issue we will monitor only sensor 0 which is the master sensor. > > > + */ > > > + > > > + /* Skip if the sensor is disabled */ > > > + if (sensor & 1) { > > > + ret = regmap_read(priv->tm_map, priv->sensor[0].status, > > &code); > > > + if (ret) > > > + return; > > > + upper_th_x = code >= threshold; > > > + lower_th_x = code <= threshold_low; > > > + if (upper_th_x) > > > + mask |= UPPER_STATUS_CLR; > > > + if (lower_th_x) > > > + mask |= LOWER_STATUS_CLR; > > > + if (upper_th_x || lower_th_x) { > > > + /* Notify user space */ > > > + schedule_work(&priv->sensor[0].notify_work); > > > + pr_debug("Trigger (%d degrees) for sensor %d\n", > > > + code_to_mdegC(code, &priv->sensor[0]), 0); > > > + } > > > + } > > > + regmap_write(priv->tm_map, STATUS_CNTL_ADDR_8064, reg & > > mask); > > > +} > > > + > > > +static irqreturn_t tsens_isr(int irq, void *data) > > > +{ > > > + struct tsens_priv *priv = data; > > > + > > > + schedule_work(&priv->tsens_work); > > > + return IRQ_HANDLED; > > > > > > Have you considered trying to reuse the regmap and interrupt handling > > infrastructure in tsens.c that I used to convert over everything after > > IP version 0.1? > > > > I started converting over 8960 but never managed to finish testing > > this[1]. I'd be happy for you to take this over and get it working so > > the 8960 doesn't end up being a completely separate driver from the > > other platforms. > > > > [1] > > https://git.linaro.org/people/amit.kucheria/kernel.git/log/?h=wrk3/tsens- > > 8960-breakage > > > > Thanks a lot for the link. I started doing some test and I think the only general > code we will be able to use will be the init_common. The get temp and > the function to convert code to decg are very different from the one used in > 8960. Do you think keep a custom get temp function is good or not? OK. Let's start common init code and custom get_temp and adc-to-degc functions. Regards, Amit
