On Sat, Aug 6, 2022 at 6:18 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > On Mon, 1 Aug 2022 11:08:39 +0200 > Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: > > On Mon, Aug 1, 2022 at 6:19 AM Matt Ranostay <matt.ranostay@xxxxxxxxxxxx> wrote: > > > On Mon, Aug 1, 2022 at 3:11 AM Andy Shevchenko > > > <andy.shevchenko@xxxxxxxxx> wrote: > > > > On Fri, Jul 29, 2022 at 5:49 PM Matt Ranostay > > > > <matt.ranostay@xxxxxxxxxxxx> wrote: ... > > > > > + if (mcp->indio_dev) > > > > > > > > Do you need this check? > > > > > > Yes basically if no ADC or DAC channel is enabled then no iio_device > > > get allocated or registered. > > > > > > > + iio_device_unregister(mcp->indio_dev); > > > > So, we have an inconvenience in the iio_device_unregister(), i.e. it > > doesn't perform the NULL-check by itself. I recommend fixing it there > > and dropping this check in the caller. This is standard practice in > > the Linux kernel for resource deallocator APIs. > > ah. Now the other patch makes more sense. Make sure to pull this > driver an that one together in a series if we are taking that forwards. > > I agree for allocator APIs but not so much for registration APIs. > I checked a few similar ones. > > input_unregister_device() doesn't > hwmon_device_unregister() doesn't. > > Got bored at that point :) Good point. I think that check is simply not needed then. We should never come to _unregister() with a NULL pointer, right? > Would be relatively easy to take this driver fully devm_ I think > then you can just use devm_iio_device_register() if you have a device > to register. -- With Best Regards, Andy Shevchenko
