| Message ID | 20210729080135.17436-2-hui.liu@mediatek.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | AUXADC: Mediatek auxadc driver | expand |
On Thu, 29 Jul 2021 16:01:35 +0800 Hui Liu <hui.liu@mediatek.com> wrote: > Add mutex_destroy when probe fail and remove device. > > Signed-off-by: Hui Liu <hui.liu@mediatek.com> Hi Hui Liu, Two things here. 1) You need to explain with a clear example flow when this would serve a useful purpose. As I explained before, we do no in general put mutex_destroy() in remove paths as it is usually just noise. 2) It's in the wrong order logically. mutex init is between the clk_prepare_enable and iio_device_register, hence if we are going to have mutex destroy it must also be in that that location (remove should be reverse of probe or there should be a clear comment explaining why we need to do things in a different order. 3) If touching this code at all, please move all of the probe / remove to devm_ managed code so that we don't need to get this ordering right at all because it will be done automatically. So I won't apply this without 1 and even if I accepted the principle, it's still in the wrong place in remove. Jonathan > --- > drivers/iio/adc/mt6577_auxadc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c > index 79c1dd68b909..d57243037ad6 100644 > --- a/drivers/iio/adc/mt6577_auxadc.c > +++ b/drivers/iio/adc/mt6577_auxadc.c > @@ -289,6 +289,7 @@ static int mt6577_auxadc_probe(struct platform_device *pdev) > ret = iio_device_register(indio_dev); > if (ret < 0) { > dev_err(&pdev->dev, "failed to register iio device\n"); > + mutex_destroy(&adc_dev->lock); > goto err_power_off; > } > > @@ -313,6 +314,7 @@ static int mt6577_auxadc_remove(struct platform_device *pdev) > 0, MT6577_AUXADC_PDN_EN); > > clk_disable_unprepare(adc_dev->adc_clk); > + mutex_destroy(&adc_dev->lock); > > return 0; > }
diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c index 79c1dd68b909..d57243037ad6 100644 --- a/drivers/iio/adc/mt6577_auxadc.c +++ b/drivers/iio/adc/mt6577_auxadc.c @@ -289,6 +289,7 @@ static int mt6577_auxadc_probe(struct platform_device *pdev) ret = iio_device_register(indio_dev); if (ret < 0) { dev_err(&pdev->dev, "failed to register iio device\n"); + mutex_destroy(&adc_dev->lock); goto err_power_off; } @@ -313,6 +314,7 @@ static int mt6577_auxadc_remove(struct platform_device *pdev) 0, MT6577_AUXADC_PDN_EN); clk_disable_unprepare(adc_dev->adc_clk); + mutex_destroy(&adc_dev->lock); return 0; }
Add mutex_destroy when probe fail and remove device. Signed-off-by: Hui Liu <hui.liu@mediatek.com> --- drivers/iio/adc/mt6577_auxadc.c | 2 ++ 1 file changed, 2 insertions(+)