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@xxxxxxxxxxxxxxxxxxxx>
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;
> };
>
> /*
