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@xxxxxxxxxxxxxxxxxxxx>

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);