Re: [PATCH 07/11] iio: adc: Add support for QCOM PMIC5 Gen3 ADC

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

 



Hi Dmitry,

On 10/23/2023 1:33 PM, Dmitry Baryshkov wrote:
On Mon, 23 Oct 2023 at 09:15, Jishnu Prakash<quic_jprakash@xxxxxxxxxxx>  wrote:
Hi Jonathan,

On 7/8/2023 9:29 PM, Jonathan Cameron wrote:
On Sat, 8 Jul 2023 12:58:31 +0530
Jishnu Prakash<quic_jprakash@xxxxxxxxxxx>  wrote:
+
+    return devm_iio_device_register(dev, indio_dev);
+}
+
+static int adc5_gen3_exit(struct platform_device *pdev)
+{
As you are mixing devm manged cleanup and the explicit sort the
result is that you remove the userspace interfaces 'after' you run
everything in here. I'm thinking disabling the channels at least
isn't a good idea in that case.

If you want to use devm (which is good) then you need to work out how
to register additional callbacks during probe to tear down everything in
the right order (typically the reverse of what happens in probe)
devm_add_action_or_reset() is the way to add those extra callbacks.

If not, just don't use devm for at least those bits that will end up
running out of order (such as iio_device_register()) and manually call their
cleanup routines instead.
I checked some other examples in the iio/adc/ folder, I think I see what
you mean here. It looks like drivers with a remove callback always use
iio_device_register and iio_device_unregister instead of the devm_*
variant, due to the issue with sysfs removal as you said.

I'll update the probe and remove functions similarly, to do explicit
cleanups as required, avoiding devm_ usage for places where it should be
avoided.
I think you got the message all wrong. There is nothing bad with using
devm_. As a matter of fact it is a preferred form in most of the
cases. However you have to be careful to tear down your device in the
correct order. And as Jonathan pointed
out, you might add necessary hooks manually by calling
devm_add_action_or_reset().

[skipped the rest]

Thanks for your comment. I checked the code again and I think we can do the teardown with a devm_add_action() call and drop the remove API entirely in favor of using devm_* APIs , I'll update this in the next patchset.

Thanks,

Jishnu







[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux