> -----Original Message-----
> From: Jonathan Cameron <jic23@xxxxxxxxxx>
> Sent: 2023年1月8日 21:15
> 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 v5 0/3] add imx93 adc support
>
> On Tue, 3 Jan 2023 19:43:55 +0800
> haibo.chen@xxxxxxx wrote:
>
> > From: Haibo Chen <haibo.chen@xxxxxxx>
> >
> > V5:
> > -For ADC driver, use dev_err_probe() to replace dev_err() in
> dev_err_probe().
> > -Add imx93_adc_power_down() in the probe error path.
> > -Re-order the function in imx93_adc_remove(), make them inverse in
> probe().
> > -Remove the pm_runtime_get_sync(dev) in imx93_adc_remove(), because
> this driver
> > enable the pm_runtime autosuspend feature, and config the delay as
> 50ms. So when
> > called imx93_adc_remove(), this device still in runtime resume state, no
> need to
> > force resume the device back.
> I don't follow this point. Perhaps talk me through in more detail on why the
> device will be in a runtime resumed state when ever we hit remove?
Hi Jonathan,
Sorry for delay.
This driver use module_platform_driver, so when do rmmod or unbind operation
The function call steps are as belowing:
platform_driver_unregister
--> driver_unregister
--> bus_remove_driver
--> driver_detach
--> device_release_driver_internal
--> __device_release_driver
In __device_release_driver {
pm_runtime_get_sync(dev);
...
pm_runtime_put_sync(dev);
device_remove(dev); -> call imx93_adc_remove()
...
}
Since in this imx93 adc driver, we use 50ms auto suspend dealy,
pm_runtime_set_autosuspend_delay(dev, 50);
and here is the description of this API (Documentation/power/runtime_pm.rst):
`void pm_runtime_set_autosuspend_delay(struct device *dev, int delay);`
- set the power.autosuspend_delay value to 'delay' (expressed in
milliseconds); if 'delay' is negative then runtime suspends are
prevented; if power.use_autosuspend is set, pm_runtime_get_sync may be
called or the device's usage counter may be decremented and
pm_runtime_idle called depending on if power.autosuspend_delay is
changed to or from a negative value; if power.use_autosuspend is clear,
pm_runtime_idle is called
and the description of pm_runtime_put_sync.
/**
* pm_runtime_put_sync - Drop device usage counter and run "idle check" if 0.
* @dev: Target device.
*
* Decrement the runtime PM usage counter of @dev and if it turns out to be
* equal to 0, invoke the "idle check" callback of @dev and, depending on its
* return value, set up autosuspend of @dev or suspend it (depending on whether
* or not autosuspend has been enabled for it).
*
* The possible return values of this function are the same as for
* pm_runtime_idle() and the runtime PM usage counter of @dev remains
* decremented in all cases, even if it returns an error code.
*/
static inline int pm_runtime_put_sync(struct device *dev)
{
return __pm_runtime_idle(dev, RPM_GET_PUT);
}
This means after call the pm_runtime_put_sync in __device_release_driver(), imx93_adc will not call imx93_adc_runtime_suspend() immediately, will do it after 50ms, but just then, call the imx93_adc_remove(), so this means when imx93_adc_remove() execute, the ADC related clocks keep on.
Best Regards
Haibo Chen
>
> > -no changes for binding doc and dts.
> >
> > V4:
> > For ADC driver, re-define the ADC status show the relation to specific
> register bit.
> > Redo the imx93_adc_remove(), change the return error sequence in
> imx93_adc_read_raw(),
> > and use a direct string for indio_dev->name.
> > For dt-bings, change the commit title and add maintainer's reviewed by
> tag
> > For dts, no change.
> >
> > V3:
> > For dt-bings, add some change according to review comments, and pass
> dt_binding_check.
> > For dts, add #io-channel-cells = <1>; to pass dtbs_check
> > For ADC driver, no change.
> >
> > V2:
> > For ADC driver, add change according to matainer's commets.
> >
> > Haibo Chen (3):
> > iio: adc: add imx93 adc support
> > dt-bindings: iio: adc: Add NXP IMX93 ADC
> > arm64: dts: imx93: add ADC support
> >
> > .../bindings/iio/adc/nxp,imx93-adc.yaml | 81 +++
> > MAINTAINERS | 4 +-
> > .../boot/dts/freescale/imx93-11x11-evk.dts | 12 +
> > arch/arm64/boot/dts/freescale/imx93.dtsi | 13 +
> > drivers/iio/adc/Kconfig | 10 +
> > drivers/iio/adc/Makefile | 1 +
> > drivers/iio/adc/imx93_adc.c | 477
> ++++++++++++++++++
> > 7 files changed, 597 insertions(+), 1 deletion(-) create mode 100644
> > Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> > create mode 100644 drivers/iio/adc/imx93_adc.c
> >
