Re: [PATCH v1 4/4] thermal: mediatek: minor mtk_thermal.c cleanups

[Louis Yu <louis.yu@mediatek.com>]

Hi Matthias,

On Thu, 2017-07-27 at 17:49 +0200, Matthias Brugger wrote:
> 
> On 07/27/2017 07:46 AM, louis.yu@mediatek.com wrote:
> > From: Louis Yu <louis.yu@mediatek.com>
> > 
> > Move thermal module reset in the beginning and revise clock error
> > handle flow to pair the clock control.
> > 
> > Signed-off-by: Louis Yu <louis.yu@mediatek.com>
> > ---
> >   drivers/thermal/mtk_thermal.c | 12 ++++++------
> >   1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> > index f2a7155..5dffdb4 100644
> > --- a/drivers/thermal/mtk_thermal.c
> > +++ b/drivers/thermal/mtk_thermal.c
> > @@ -712,20 +712,20 @@ static int mtk_thermal_probe(struct platform_device *pdev)
> >   		return -EINVAL;
> >   	}
> >   
> > +	ret = device_reset(&pdev->dev);
> > +	if (ret)
> > +		return ret;
> > +
> >   	ret = clk_prepare_enable(mt->clk_auxadc);
> >   	if (ret) {
> >   		dev_err(&pdev->dev, "Can't enable auxadc clk: %d\n", ret);
> > -		return ret;
> > -	}
> > -
> > -	ret = device_reset(&pdev->dev);
> > -	if (ret)
> >   		goto err_disable_clk_auxadc;
> > +	}
> >   
> >   	ret = clk_prepare_enable(mt->clk_peri_therm);
> >   	if (ret) {
> >   		dev_err(&pdev->dev, "Can't enable peri clk: %d\n", ret);
> > -		goto err_disable_clk_auxadc;
> > +		goto err_disable_clk_peri_therm;
> 
> If you can't enable the clock, there is no use in disable it in the error path.
> 
> Regards,
> Matthias
> 

Thanks for your reminder.This is no use to disable it in the error path.
I would update this in next version as follows.

diff --git a/drivers/thermal/mtk_thermal.c
b/drivers/thermal/mtk_thermal.c
index 5dffdb4..3df0de3 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -719,13 +719,13 @@ static int mtk_thermal_probe(struct
platform_device *pdev)
 	ret = clk_prepare_enable(mt->clk_auxadc);
 	if (ret) {
 		dev_err(&pdev->dev, "Can't enable auxadc clk: %d\n", ret);
-		goto err_disable_clk_auxadc;
+		return ret;
 	}
 
 	ret = clk_prepare_enable(mt->clk_peri_therm);
 	if (ret) {
 		dev_err(&pdev->dev, "Can't enable peri clk: %d\n", ret);
-		goto err_disable_clk_peri_therm;
+		goto err_disable_clk_auxadc;
 	}
 
 	for (i = 0; i < mt->conf->num_banks; i++)


Regards
Louis

> >   	}
> >   
> >   	for (i = 0; i < mt->conf->num_banks; i++)
> >