RE: [PATCH 1/3] iio: adc: add imx93 adc support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----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;
> > +}
> > +





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux