> -----Original Message-----
> From: Jonathan Cameron <jic23@xxxxxxxxxx>
> Sent: 2022年8月7日 0:03
> To: Bough Chen <haibo.chen@xxxxxxx>
> Cc: lars@xxxxxxxxxx; robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx;
> shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx;
> festevam@xxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>;
> linux-iio@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 1/3] iio: adc: add imx93 adc support
>
> On Wed, 3 Aug 2022 17:12:25 +0800
> haibo.chen@xxxxxxx wrote:
>
> > From: Haibo Chen <haibo.chen@xxxxxxx>
> >
> > The ADC in i.mx93 is a total new ADC IP, add a driver to support this
> > ADC.
> >
> > Currently, only support one shot normal conversion triggered by
> > software. For other mode, will add in future.
> >
> > Signed-off-by: Haibo Chen <haibo.chen@xxxxxxx>
>
> Nice.
>
> A few comments inline.
>
> Thanks,
>
> Jonathan
>
>
> > diff --git a/drivers/iio/adc/imx93_adc.c b/drivers/iio/adc/imx93_adc.c
> > new file mode 100644 index 000000000000..6e3cf6d3e629
> > --- /dev/null
> > +++ b/drivers/iio/adc/imx93_adc.c
> > @@ -0,0 +1,448 @@
>
> ..
.
...
> > +static int imx93_adc_remove(struct platform_device *pdev) {
> > + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> > + struct imx93_adc *adc = iio_priv(indio_dev);
> > + struct device *dev = adc->dev;
> > +
> > + pm_runtime_get_sync(dev);
>
> I would prefer to see the runtime pm tear down all up here as it corresponds
> to the setup at the end of probe() I don't it will make any functional difference
> and it makes the code easier to reason about in terms of ordering of calls.
Hi Jonathan,
I can't get your point here, you mean call pm_runtime_disable and pm_runtime_put_noidle here?
I get sync the runtime pm here, because there are register operation in imx93_adc_power_down(), need to make sure clock is on.
Best Regards
Haibo Chen
>
> > +
> > + iio_device_unregister(indio_dev);
> > + imx93_adc_power_down(adc);
> > + clk_disable_unprepare(adc->ipg_clk);
> > + regulator_disable(adc->vref);
> > +
> > + pm_runtime_disable(dev);
> > + pm_runtime_put_noidle(dev);
> > +
> > + return 0;
> > +}
> > +
