[PATCH 0/3] hwmon: bt1-pvt: Fix PVT sensor being unpowered

[PATCH 0/3] hwmon: bt1-pvt: Fix PVT sensor being unpowered

[Serge Semin]

Baikal-T1 PVT sensor has got a dedicated power domain with 1.8V intended
to be supplied via the external GPVT and VPVT_18 pins. In case if the
power isn't supplied, the sensor IO registers will be accessible, but the
data conversion just won't happen. The situation of the power loss
currently will cause the temperature and voltage read procedures to hang
up. The problem is fixed in the framework of this patchset firstly by
checking whether the conversion works at the sensor probe procedure, and
secondly by keeping the current conversion update timeout in the private
driver data cache and using it to set the wait-completion timeout.

Fixes: 87976ce2825d ("hwmon: Add Baikal-T1 PVT sensor driver")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
Cc: linux-mips@vger.kernel.org
Cc: linux-hwmon@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Serge Semin (3):
  hwmon: bt1-pvt: Test sensor power supply on probe
  hwmon: bt1-pvt: Cache current update timeout
  hwmon: bt1-pvt: Wait for the completion with timeout

 drivers/hwmon/bt1-pvt.c | 138 ++++++++++++++++++++++++++++------------
 drivers/hwmon/bt1-pvt.h |   3 +
 2 files changed, 101 insertions(+), 40 deletions(-)

-- 
2.27.0

[PATCH 1/3] hwmon: bt1-pvt: Test sensor power supply on probe

[Serge Semin]

Baikal-T1 PVT sensor has got a dedicated power supply domain (feed up by
the external GPVT/VPVT_18 pins). In case if it isn't powered up, the
registers will be accessible, but the sensor conversion just won't happen.
Due to that an attempt to read data from any PVT sensor will cause the
task hanging up.  For instance that will happen if XP11 jumper isn't
installed on the Baikal-T1-based BFK3.1 board. Let's at least test whether
the conversion work on the device probe procedure. By doing so will make
sure that the PVT sensor is powered up at least at boot time.

Fixes: 87976ce2825d ("hwmon: Add Baikal-T1 PVT sensor driver")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/hwmon/bt1-pvt.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/hwmon/bt1-pvt.c b/drivers/hwmon/bt1-pvt.c
index 94698cae0497..f4b7353c078a 100644
--- a/drivers/hwmon/bt1-pvt.c
+++ b/drivers/hwmon/bt1-pvt.c
@@ -13,6 +13,7 @@
 #include <linux/bitops.h>
 #include <linux/clk.h>
 #include <linux/completion.h>
+#include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/hwmon-sysfs.h>
 #include <linux/hwmon.h>
@@ -982,6 +983,41 @@ static int pvt_request_clks(struct pvt_hwmon *pvt)
 	return 0;
 }
 
+static int pvt_check_pwr(struct pvt_hwmon *pvt)
+{
+	unsigned long tout;
+	int ret = 0;
+	u32 data;
+
+	/*
+	 * Test out the sensor conversion functionality. If it is not done on
+	 * time then the domain must have been unpowered and we won't be able
+	 * to use the device later in this driver.
+	 * Note If the power source is lost during the normal driver work the
+	 * data read procedure will either return -ETIMEDOUT (for the
+	 * alarm-less driver configuration) or just stop the repeated
+	 * conversion. In the later case alas we won't be able to detect the
+	 * problem.
+	 */
+	pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_ALL, PVT_INTR_ALL);
+	pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, PVT_CTRL_EN);
+	pvt_set_tout(pvt, 0);
+	readl(pvt->regs + PVT_DATA);
+
+	tout = PVT_TOUT_MIN / NSEC_PER_USEC;
+	usleep_range(tout, 2 * tout);
+
+	data = readl(pvt->regs + PVT_DATA);
+	if (!(data & PVT_DATA_VALID)) {
+		ret = -ENODEV;
+		dev_err(pvt->dev, "Sensor is powered down\n");
+	}
+
+	pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, 0);
+
+	return ret;
+}
+
 static void pvt_init_iface(struct pvt_hwmon *pvt)
 {
 	u32 trim, temp;
@@ -1109,6 +1145,10 @@ static int pvt_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	ret = pvt_check_pwr(pvt);
+	if (ret)
+		return ret;
+
 	pvt_init_iface(pvt);
 
 	ret = pvt_request_irq(pvt);
-- 
2.27.0

[PATCH 2/3] hwmon: bt1-pvt: Cache current update timeout

[Serge Semin]

Instead of converting the update timeout data to the milliseconds each
time on the read procedure let's preserve the currently set timeout in the
dedicated driver private data cache. The cached value will be then used in
the timeout read method and in the alarm-less data conversion to prevent
the caller task hanging up in case if the PVT sensor is suddenly powered
down.

Fixes: 87976ce2825d ("hwmon: Add Baikal-T1 PVT sensor driver")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/hwmon/bt1-pvt.c | 85 ++++++++++++++++++++++-------------------
 drivers/hwmon/bt1-pvt.h |  3 ++
 2 files changed, 49 insertions(+), 39 deletions(-)

diff --git a/drivers/hwmon/bt1-pvt.c b/drivers/hwmon/bt1-pvt.c
index f4b7353c078a..2600426a3b21 100644
--- a/drivers/hwmon/bt1-pvt.c
+++ b/drivers/hwmon/bt1-pvt.c
@@ -655,44 +655,16 @@ static int pvt_write_trim(struct pvt_hwmon *pvt, long val)
 
 static int pvt_read_timeout(struct pvt_hwmon *pvt, long *val)
 {
-	unsigned long rate;
-	ktime_t kt;
-	u32 data;
-
-	rate = clk_get_rate(pvt->clks[PVT_CLOCK_REF].clk);
-	if (!rate)
-		return -ENODEV;
-
-	/*
-	 * Don't bother with mutex here, since we just read data from MMIO.
-	 * We also have to scale the ticks timeout up to compensate the
-	 * ms-ns-data translations.
-	 */
-	data = readl(pvt->regs + PVT_TTIMEOUT) + 1;
+	int ret;
 
-	/*
-	 * Calculate ref-clock based delay (Ttotal) between two consecutive
-	 * data samples of the same sensor. So we first must calculate the
-	 * delay introduced by the internal ref-clock timer (Tref * Fclk).
-	 * Then add the constant timeout cuased by each conversion latency
-	 * (Tmin). The basic formulae for each conversion is following:
-	 *   Ttotal = Tref * Fclk + Tmin
-	 * Note if alarms are enabled the sensors are polled one after
-	 * another, so in order to have the delay being applicable for each
-	 * sensor the requested value must be equally redistirbuted.
-	 */
-#if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
-	kt = ktime_set(PVT_SENSORS_NUM * (u64)data, 0);
-	kt = ktime_divns(kt, rate);
-	kt = ktime_add_ns(kt, PVT_SENSORS_NUM * PVT_TOUT_MIN);
-#else
-	kt = ktime_set(data, 0);
-	kt = ktime_divns(kt, rate);
-	kt = ktime_add_ns(kt, PVT_TOUT_MIN);
-#endif
+	ret = mutex_lock_interruptible(&pvt->iface_mtx);
+	if (ret)
+		return ret;
 
 	/* Return the result in msec as hwmon sysfs interface requires. */
-	*val = ktime_to_ms(kt);
+	*val = ktime_to_ms(pvt->timeout);
+
+	mutex_unlock(&pvt->iface_mtx);
 
 	return 0;
 }
@@ -700,7 +672,7 @@ static int pvt_read_timeout(struct pvt_hwmon *pvt, long *val)
 static int pvt_write_timeout(struct pvt_hwmon *pvt, long val)
 {
 	unsigned long rate;
-	ktime_t kt;
+	ktime_t kt, cache;
 	u32 data;
 	int ret;
 
@@ -713,7 +685,7 @@ static int pvt_write_timeout(struct pvt_hwmon *pvt, long val)
 	 * between all available sensors to have the requested delay
 	 * applicable to each individual sensor.
 	 */
-	kt = ms_to_ktime(val);
+	cache = kt = ms_to_ktime(val);
 #if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
 	kt = ktime_divns(kt, PVT_SENSORS_NUM);
 #endif
@@ -742,6 +714,7 @@ static int pvt_write_timeout(struct pvt_hwmon *pvt, long val)
 		return ret;
 
 	pvt_set_tout(pvt, data);
+	pvt->timeout = cache;
 
 	mutex_unlock(&pvt->iface_mtx);
 
@@ -1018,10 +991,17 @@ static int pvt_check_pwr(struct pvt_hwmon *pvt)
 	return ret;
 }
 
-static void pvt_init_iface(struct pvt_hwmon *pvt)
+static int pvt_init_iface(struct pvt_hwmon *pvt)
 {
+	unsigned long rate;
 	u32 trim, temp;
 
+	rate = clk_get_rate(pvt->clks[PVT_CLOCK_REF].clk);
+	if (!rate) {
+		dev_err(pvt->dev, "Invalid reference clock rate\n");
+		return -ENODEV;
+	}
+
 	/*
 	 * Make sure all interrupts and controller are disabled so not to
 	 * accidentally have ISR executed before the driver data is fully
@@ -1036,12 +1016,37 @@ static void pvt_init_iface(struct pvt_hwmon *pvt)
 	pvt_set_mode(pvt, pvt_info[pvt->sensor].mode);
 	pvt_set_tout(pvt, PVT_TOUT_DEF);
 
+	/*
+	 * Preserve the current ref-clock based delay (Ttotal) between the
+	 * sensors data samples in the driver data so not to recalculate it
+	 * each time on the data requests and timeout reads. It consists of the
+	 * delay introduced by the internal ref-clock timer (N / Fclk) and the
+	 * constant timeout caused by each conversion latency (Tmin):
+	 *   Ttotal = N / Fclk + Tmin
+	 * If alarms are enabled the sensors are polled one after another and
+	 * in order to get the next measurement of a particular sensor the
+	 * caller will have to wait for at most until all the others are
+	 * polled. In that case the formulae will look a bit different:
+	 *   Ttotal = 5 * (N / Fclk + Tmin)
+	 */
+#if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
+	pvt->timeout = ktime_set(PVT_SENSORS_NUM * PVT_TOUT_DEF, 0);
+	pvt->timeout = ktime_divns(pvt->timeout, rate);
+	pvt->timeout = ktime_add_ns(pvt->timeout, PVT_SENSORS_NUM * PVT_TOUT_MIN);
+#else
+	pvt->timeout = ktime_set(PVT_TOUT_DEF, 0);
+	pvt->timeout = ktime_divns(pvt->timeout, rate);
+	pvt->timeout = ktime_add_ns(pvt->timeout, PVT_TOUT_MIN);
+#endif
+
 	trim = PVT_TRIM_DEF;
 	if (!of_property_read_u32(pvt->dev->of_node,
 	     "baikal,pvt-temp-offset-millicelsius", &temp))
 		trim = pvt_calc_trim(temp);
 
 	pvt_set_trim(pvt, trim);
+
+	return 0;
 }
 
 static int pvt_request_irq(struct pvt_hwmon *pvt)
@@ -1149,7 +1154,9 @@ static int pvt_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	pvt_init_iface(pvt);
+	ret = pvt_init_iface(pvt);
+	if (ret)
+		return ret;
 
 	ret = pvt_request_irq(pvt);
 	if (ret)
diff --git a/drivers/hwmon/bt1-pvt.h b/drivers/hwmon/bt1-pvt.h
index 5eac73e94885..93b8dd5e7c94 100644
--- a/drivers/hwmon/bt1-pvt.h
+++ b/drivers/hwmon/bt1-pvt.h
@@ -10,6 +10,7 @@
 #include <linux/completion.h>
 #include <linux/hwmon.h>
 #include <linux/kernel.h>
+#include <linux/ktime.h>
 #include <linux/mutex.h>
 #include <linux/seqlock.h>
 
@@ -201,6 +202,7 @@ struct pvt_cache {
  *	       if alarms are disabled).
  * @sensor: current PVT sensor the data conversion is being performed for.
  * @cache: data cache descriptor.
+ * @timeout: conversion timeout cache.
  */
 struct pvt_hwmon {
 	struct device *dev;
@@ -214,6 +216,7 @@ struct pvt_hwmon {
 	struct mutex iface_mtx;
 	enum pvt_sensor_type sensor;
 	struct pvt_cache cache[PVT_SENSORS_NUM];
+	ktime_t timeout;
 };
 
 /*
-- 
2.27.0

[PATCH 3/3] hwmon: bt1-pvt: Wait for the completion with timeout

[Serge Semin]

If the PVT sensor is suddenly powered down while a caller is waiting for
the conversion completion, the request won't be finished and the task will
hang up on this procedure until the power is back up again. Let's call the
wait_for_completion_timeout() method instead to prevent that. The cached
timeout is exactly what we need to predict for how long conversion could
normally last.

Fixes: 87976ce2825d ("hwmon: Add Baikal-T1 PVT sensor driver")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/hwmon/bt1-pvt.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/bt1-pvt.c b/drivers/hwmon/bt1-pvt.c
index 2600426a3b21..3e1d56585b91 100644
--- a/drivers/hwmon/bt1-pvt.c
+++ b/drivers/hwmon/bt1-pvt.c
@@ -477,6 +477,7 @@ static int pvt_read_data(struct pvt_hwmon *pvt, enum pvt_sensor_type type,
 			 long *val)
 {
 	struct pvt_cache *cache = &pvt->cache[type];
+	unsigned long timeout;
 	u32 data;
 	int ret;
 
@@ -500,7 +501,14 @@ static int pvt_read_data(struct pvt_hwmon *pvt, enum pvt_sensor_type type,
 	pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_DVALID, 0);
 	pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, PVT_CTRL_EN);
 
-	wait_for_completion(&cache->conversion);
+	/*
+	 * Wait with timeout since in case if the sensor is suddenly powered
+	 * down the request won't be completed and the caller will hang up on
+	 * this procedure until the power is back up again. Multiply the
+	 * timeout by the factor of two to prevent a false timeout.
+	 */
+	timeout = 2 * usecs_to_jiffies(ktime_to_us(pvt->timeout));
+	ret = wait_for_completion_timeout(&cache->conversion, timeout);
 
 	pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, 0);
 	pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_DVALID,
@@ -510,6 +518,9 @@ static int pvt_read_data(struct pvt_hwmon *pvt, enum pvt_sensor_type type,
 
 	mutex_unlock(&pvt->iface_mtx);
 
+	if (!ret)
+		return -ETIMEDOUT;
+
 	if (type == PVT_TEMP)
 		*val = pvt_calc_poly(&poly_N_to_temp, data);
 	else
-- 
2.27.0

Re: [PATCH 1/3] hwmon: bt1-pvt: Test sensor power supply on probe

[Guenter Roeck]

On Sun, Sep 20, 2020 at 02:09:21PM +0300, Serge Semin wrote:
> Baikal-T1 PVT sensor has got a dedicated power supply domain (feed up by
> the external GPVT/VPVT_18 pins). In case if it isn't powered up, the
> registers will be accessible, but the sensor conversion just won't happen.
> Due to that an attempt to read data from any PVT sensor will cause the
> task hanging up.  For instance that will happen if XP11 jumper isn't
> installed on the Baikal-T1-based BFK3.1 board. Let's at least test whether
> the conversion work on the device probe procedure. By doing so will make
> sure that the PVT sensor is powered up at least at boot time.
> 
> Fixes: 87976ce2825d ("hwmon: Add Baikal-T1 PVT sensor driver")
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/bt1-pvt.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/hwmon/bt1-pvt.c b/drivers/hwmon/bt1-pvt.c
> index 94698cae0497..f4b7353c078a 100644
> --- a/drivers/hwmon/bt1-pvt.c
> +++ b/drivers/hwmon/bt1-pvt.c
> @@ -13,6 +13,7 @@
>  #include <linux/bitops.h>
>  #include <linux/clk.h>
>  #include <linux/completion.h>
> +#include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/hwmon-sysfs.h>
>  #include <linux/hwmon.h>
> @@ -982,6 +983,41 @@ static int pvt_request_clks(struct pvt_hwmon *pvt)
>  	return 0;
>  }
>  
> +static int pvt_check_pwr(struct pvt_hwmon *pvt)
> +{
> +	unsigned long tout;
> +	int ret = 0;
> +	u32 data;
> +
> +	/*
> +	 * Test out the sensor conversion functionality. If it is not done on
> +	 * time then the domain must have been unpowered and we won't be able
> +	 * to use the device later in this driver.
> +	 * Note If the power source is lost during the normal driver work the
> +	 * data read procedure will either return -ETIMEDOUT (for the
> +	 * alarm-less driver configuration) or just stop the repeated
> +	 * conversion. In the later case alas we won't be able to detect the
> +	 * problem.
> +	 */
> +	pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_ALL, PVT_INTR_ALL);
> +	pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, PVT_CTRL_EN);
> +	pvt_set_tout(pvt, 0);
> +	readl(pvt->regs + PVT_DATA);
> +
> +	tout = PVT_TOUT_MIN / NSEC_PER_USEC;
> +	usleep_range(tout, 2 * tout);
> +
> +	data = readl(pvt->regs + PVT_DATA);
> +	if (!(data & PVT_DATA_VALID)) {
> +		ret = -ENODEV;
> +		dev_err(pvt->dev, "Sensor is powered down\n");
> +	}
> +
> +	pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, 0);
> +
> +	return ret;
> +}
> +
>  static void pvt_init_iface(struct pvt_hwmon *pvt)
>  {
>  	u32 trim, temp;
> @@ -1109,6 +1145,10 @@ static int pvt_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	ret = pvt_check_pwr(pvt);
> +	if (ret)
> +		return ret;
> +
>  	pvt_init_iface(pvt);
>  
>  	ret = pvt_request_irq(pvt);

Re: [PATCH 2/3] hwmon: bt1-pvt: Cache current update timeout

[Guenter Roeck]

On Sun, Sep 20, 2020 at 02:09:22PM +0300, Serge Semin wrote:
> Instead of converting the update timeout data to the milliseconds each
> time on the read procedure let's preserve the currently set timeout in the
> dedicated driver private data cache. The cached value will be then used in
> the timeout read method and in the alarm-less data conversion to prevent
> the caller task hanging up in case if the PVT sensor is suddenly powered
> down.
> 
> Fixes: 87976ce2825d ("hwmon: Add Baikal-T1 PVT sensor driver")
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/bt1-pvt.c | 85 ++++++++++++++++++++++-------------------
>  drivers/hwmon/bt1-pvt.h |  3 ++
>  2 files changed, 49 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/hwmon/bt1-pvt.c b/drivers/hwmon/bt1-pvt.c
> index f4b7353c078a..2600426a3b21 100644
> --- a/drivers/hwmon/bt1-pvt.c
> +++ b/drivers/hwmon/bt1-pvt.c
> @@ -655,44 +655,16 @@ static int pvt_write_trim(struct pvt_hwmon *pvt, long val)
>  
>  static int pvt_read_timeout(struct pvt_hwmon *pvt, long *val)
>  {
> -	unsigned long rate;
> -	ktime_t kt;
> -	u32 data;
> -
> -	rate = clk_get_rate(pvt->clks[PVT_CLOCK_REF].clk);
> -	if (!rate)
> -		return -ENODEV;
> -
> -	/*
> -	 * Don't bother with mutex here, since we just read data from MMIO.
> -	 * We also have to scale the ticks timeout up to compensate the
> -	 * ms-ns-data translations.
> -	 */
> -	data = readl(pvt->regs + PVT_TTIMEOUT) + 1;
> +	int ret;
>  
> -	/*
> -	 * Calculate ref-clock based delay (Ttotal) between two consecutive
> -	 * data samples of the same sensor. So we first must calculate the
> -	 * delay introduced by the internal ref-clock timer (Tref * Fclk).
> -	 * Then add the constant timeout cuased by each conversion latency
> -	 * (Tmin). The basic formulae for each conversion is following:
> -	 *   Ttotal = Tref * Fclk + Tmin
> -	 * Note if alarms are enabled the sensors are polled one after
> -	 * another, so in order to have the delay being applicable for each
> -	 * sensor the requested value must be equally redistirbuted.
> -	 */
> -#if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> -	kt = ktime_set(PVT_SENSORS_NUM * (u64)data, 0);
> -	kt = ktime_divns(kt, rate);
> -	kt = ktime_add_ns(kt, PVT_SENSORS_NUM * PVT_TOUT_MIN);
> -#else
> -	kt = ktime_set(data, 0);
> -	kt = ktime_divns(kt, rate);
> -	kt = ktime_add_ns(kt, PVT_TOUT_MIN);
> -#endif
> +	ret = mutex_lock_interruptible(&pvt->iface_mtx);
> +	if (ret)
> +		return ret;
>  
>  	/* Return the result in msec as hwmon sysfs interface requires. */
> -	*val = ktime_to_ms(kt);
> +	*val = ktime_to_ms(pvt->timeout);
> +
> +	mutex_unlock(&pvt->iface_mtx);
>  
>  	return 0;
>  }
> @@ -700,7 +672,7 @@ static int pvt_read_timeout(struct pvt_hwmon *pvt, long *val)
>  static int pvt_write_timeout(struct pvt_hwmon *pvt, long val)
>  {
>  	unsigned long rate;
> -	ktime_t kt;
> +	ktime_t kt, cache;
>  	u32 data;
>  	int ret;
>  
> @@ -713,7 +685,7 @@ static int pvt_write_timeout(struct pvt_hwmon *pvt, long val)
>  	 * between all available sensors to have the requested delay
>  	 * applicable to each individual sensor.
>  	 */
> -	kt = ms_to_ktime(val);
> +	cache = kt = ms_to_ktime(val);
>  #if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
>  	kt = ktime_divns(kt, PVT_SENSORS_NUM);
>  #endif
> @@ -742,6 +714,7 @@ static int pvt_write_timeout(struct pvt_hwmon *pvt, long val)
>  		return ret;
>  
>  	pvt_set_tout(pvt, data);
> +	pvt->timeout = cache;
>  
>  	mutex_unlock(&pvt->iface_mtx);
>  
> @@ -1018,10 +991,17 @@ static int pvt_check_pwr(struct pvt_hwmon *pvt)
>  	return ret;
>  }
>  
> -static void pvt_init_iface(struct pvt_hwmon *pvt)
> +static int pvt_init_iface(struct pvt_hwmon *pvt)
>  {
> +	unsigned long rate;
>  	u32 trim, temp;
>  
> +	rate = clk_get_rate(pvt->clks[PVT_CLOCK_REF].clk);
> +	if (!rate) {
> +		dev_err(pvt->dev, "Invalid reference clock rate\n");
> +		return -ENODEV;
> +	}
> +
>  	/*
>  	 * Make sure all interrupts and controller are disabled so not to
>  	 * accidentally have ISR executed before the driver data is fully
> @@ -1036,12 +1016,37 @@ static void pvt_init_iface(struct pvt_hwmon *pvt)
>  	pvt_set_mode(pvt, pvt_info[pvt->sensor].mode);
>  	pvt_set_tout(pvt, PVT_TOUT_DEF);
>  
> +	/*
> +	 * Preserve the current ref-clock based delay (Ttotal) between the
> +	 * sensors data samples in the driver data so not to recalculate it
> +	 * each time on the data requests and timeout reads. It consists of the
> +	 * delay introduced by the internal ref-clock timer (N / Fclk) and the
> +	 * constant timeout caused by each conversion latency (Tmin):
> +	 *   Ttotal = N / Fclk + Tmin
> +	 * If alarms are enabled the sensors are polled one after another and
> +	 * in order to get the next measurement of a particular sensor the
> +	 * caller will have to wait for at most until all the others are
> +	 * polled. In that case the formulae will look a bit different:
> +	 *   Ttotal = 5 * (N / Fclk + Tmin)
> +	 */
> +#if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> +	pvt->timeout = ktime_set(PVT_SENSORS_NUM * PVT_TOUT_DEF, 0);
> +	pvt->timeout = ktime_divns(pvt->timeout, rate);
> +	pvt->timeout = ktime_add_ns(pvt->timeout, PVT_SENSORS_NUM * PVT_TOUT_MIN);
> +#else
> +	pvt->timeout = ktime_set(PVT_TOUT_DEF, 0);
> +	pvt->timeout = ktime_divns(pvt->timeout, rate);
> +	pvt->timeout = ktime_add_ns(pvt->timeout, PVT_TOUT_MIN);
> +#endif
> +
>  	trim = PVT_TRIM_DEF;
>  	if (!of_property_read_u32(pvt->dev->of_node,
>  	     "baikal,pvt-temp-offset-millicelsius", &temp))
>  		trim = pvt_calc_trim(temp);
>  
>  	pvt_set_trim(pvt, trim);
> +
> +	return 0;
>  }
>  
>  static int pvt_request_irq(struct pvt_hwmon *pvt)
> @@ -1149,7 +1154,9 @@ static int pvt_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	pvt_init_iface(pvt);
> +	ret = pvt_init_iface(pvt);
> +	if (ret)
> +		return ret;
>  
>  	ret = pvt_request_irq(pvt);
>  	if (ret)
> diff --git a/drivers/hwmon/bt1-pvt.h b/drivers/hwmon/bt1-pvt.h
> index 5eac73e94885..93b8dd5e7c94 100644
> --- a/drivers/hwmon/bt1-pvt.h
> +++ b/drivers/hwmon/bt1-pvt.h
> @@ -10,6 +10,7 @@
>  #include <linux/completion.h>
>  #include <linux/hwmon.h>
>  #include <linux/kernel.h>
> +#include <linux/ktime.h>
>  #include <linux/mutex.h>
>  #include <linux/seqlock.h>
>  
> @@ -201,6 +202,7 @@ struct pvt_cache {
>   *	       if alarms are disabled).
>   * @sensor: current PVT sensor the data conversion is being performed for.
>   * @cache: data cache descriptor.
> + * @timeout: conversion timeout cache.
>   */
>  struct pvt_hwmon {
>  	struct device *dev;
> @@ -214,6 +216,7 @@ struct pvt_hwmon {
>  	struct mutex iface_mtx;
>  	enum pvt_sensor_type sensor;
>  	struct pvt_cache cache[PVT_SENSORS_NUM];
> +	ktime_t timeout;
>  };
>  
>  /*

Re: [PATCH 3/3] hwmon: bt1-pvt: Wait for the completion with timeout

[Guenter Roeck]

On Sun, Sep 20, 2020 at 02:09:23PM +0300, Serge Semin wrote:
> If the PVT sensor is suddenly powered down while a caller is waiting for
> the conversion completion, the request won't be finished and the task will
> hang up on this procedure until the power is back up again. Let's call the
> wait_for_completion_timeout() method instead to prevent that. The cached
> timeout is exactly what we need to predict for how long conversion could
> normally last.
> 
> Fixes: 87976ce2825d ("hwmon: Add Baikal-T1 PVT sensor driver")
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/bt1-pvt.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/bt1-pvt.c b/drivers/hwmon/bt1-pvt.c
> index 2600426a3b21..3e1d56585b91 100644
> --- a/drivers/hwmon/bt1-pvt.c
> +++ b/drivers/hwmon/bt1-pvt.c
> @@ -477,6 +477,7 @@ static int pvt_read_data(struct pvt_hwmon *pvt, enum pvt_sensor_type type,
>  			 long *val)
>  {
>  	struct pvt_cache *cache = &pvt->cache[type];
> +	unsigned long timeout;
>  	u32 data;
>  	int ret;
>  
> @@ -500,7 +501,14 @@ static int pvt_read_data(struct pvt_hwmon *pvt, enum pvt_sensor_type type,
>  	pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_DVALID, 0);
>  	pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, PVT_CTRL_EN);
>  
> -	wait_for_completion(&cache->conversion);
> +	/*
> +	 * Wait with timeout since in case if the sensor is suddenly powered
> +	 * down the request won't be completed and the caller will hang up on
> +	 * this procedure until the power is back up again. Multiply the
> +	 * timeout by the factor of two to prevent a false timeout.
> +	 */
> +	timeout = 2 * usecs_to_jiffies(ktime_to_us(pvt->timeout));
> +	ret = wait_for_completion_timeout(&cache->conversion, timeout);
>  
>  	pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, 0);
>  	pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_DVALID,
> @@ -510,6 +518,9 @@ static int pvt_read_data(struct pvt_hwmon *pvt, enum pvt_sensor_type type,
>  
>  	mutex_unlock(&pvt->iface_mtx);
>  
> +	if (!ret)
> +		return -ETIMEDOUT;
> +
>  	if (type == PVT_TEMP)
>  		*val = pvt_calc_poly(&poly_N_to_temp, data);
>  	else