[bug report] hwmon: Add hardware monitoring driver for Moortec MR75203 PVT controller

[bug report] hwmon: Add hardware monitoring driver for Moortec MR75203 PVT controller

[Dan Carpenter]

Hello Rahul Tanwar,

The patch 9d823351a337: "hwmon: Add hardware monitoring driver for
Moortec MR75203 PVT controller" from Oct 5, 2020, leads to the
following static checker warning:

	drivers/hwmon/mr75203.c:479 pvt_clk_enable()
	warn: 'pvt->clk' not released on lines: 479.

drivers/hwmon/mr75203.c
   471  static int pvt_clk_enable(struct device *dev, struct pvt_device *pvt)
   472  {
   473          int ret;
   474  
   475          ret = clk_prepare_enable(pvt->clk);
   476          if (ret)
   477                  return ret;
   478  
   479          return devm_add_action_or_reset(dev, pvt_clk_disable, pvt);

Do we have to disable the clock if devm_add_action_or_reset() fails?
This is sort of a new Smatch check so I'm not entirely sure of the rules
mysql.

   480  }

regards,
dan carpenter

Re: [bug report] hwmon: Add hardware monitoring driver for Moortec MR75203 PVT controller

[Guenter Roeck]

On Tue, Oct 13, 2020 at 05:08:58PM +0300, Dan Carpenter wrote:
> Hello Rahul Tanwar,
> 
> The patch 9d823351a337: "hwmon: Add hardware monitoring driver for
> Moortec MR75203 PVT controller" from Oct 5, 2020, leads to the
> following static checker warning:
> 
> 	drivers/hwmon/mr75203.c:479 pvt_clk_enable()
> 	warn: 'pvt->clk' not released on lines: 479.
> 
> drivers/hwmon/mr75203.c
>    471  static int pvt_clk_enable(struct device *dev, struct pvt_device *pvt)
>    472  {
>    473          int ret;
>    474  
>    475          ret = clk_prepare_enable(pvt->clk);
>    476          if (ret)
>    477                  return ret;
>    478  
>    479          return devm_add_action_or_reset(dev, pvt_clk_disable, pvt);
> 
> Do we have to disable the clock if devm_add_action_or_reset() fails?
> This is sort of a new Smatch check so I'm not entirely sure of the rules
> mysql.

devm_add_action_or_reset calls the callback function (here:
pvt_clk_disable) if there is an error, and the callback function
releases the clock. This is a workaround for the missing
devm_clk_prepare_enable(); you'll find it all over the kernel
by now.

Guenter

> 
>    480  }
> 
> regards,
> dan carpenter

Re: [bug report] hwmon: Add hardware monitoring driver for Moortec MR75203 PVT controller

[Dan Carpenter]

On Tue, Oct 13, 2020 at 09:57:14AM -0700, Guenter Roeck wrote:
> On Tue, Oct 13, 2020 at 05:08:58PM +0300, Dan Carpenter wrote:
> > Hello Rahul Tanwar,
> > 
> > The patch 9d823351a337: "hwmon: Add hardware monitoring driver for
> > Moortec MR75203 PVT controller" from Oct 5, 2020, leads to the
> > following static checker warning:
> > 
> > 	drivers/hwmon/mr75203.c:479 pvt_clk_enable()
> > 	warn: 'pvt->clk' not released on lines: 479.
> > 
> > drivers/hwmon/mr75203.c
> >    471  static int pvt_clk_enable(struct device *dev, struct pvt_device *pvt)
> >    472  {
> >    473          int ret;
> >    474  
> >    475          ret = clk_prepare_enable(pvt->clk);
> >    476          if (ret)
> >    477                  return ret;
> >    478  
> >    479          return devm_add_action_or_reset(dev, pvt_clk_disable, pvt);
> > 
> > Do we have to disable the clock if devm_add_action_or_reset() fails?
> > This is sort of a new Smatch check so I'm not entirely sure of the rules
> > mysql.
> 
> devm_add_action_or_reset calls the callback function (here:
> pvt_clk_disable) if there is an error, and the callback function
> releases the clock. This is a workaround for the missing
> devm_clk_prepare_enable(); you'll find it all over the kernel
> by now.

Ah right.  I have seen that before, yes.

regards,
dan carpenter