On lis 03, 2018 12:21, Jonathan Cameron wrote: > On Mon, 29 Oct 2018 17:52:41 +0100 > Slawomir Stepien <sst@xxxxxxxxx> wrote: > > > devm_* APIs are device managed and make code simpler. > > > > Signed-off-by: Slawomir Stepien <sst@xxxxxxxxx> > Very nearly perfect (I think). > > But there is one path where we don't quite manage to clean everything up. Or maybe more than on. See below and in v5. > > --- > Also, I should be seeing a version log here to avoid me having to look back > at previous versions (potentially) to remind me what needed changing. I am so sorry about that. Will add the whole history in v5. > > @@ -909,65 +917,41 @@ static int ad7280_probe(struct spi_device *spi) > > > > ret = ad7280_attr_init(st); > > if (ret < 0) > > - goto error_free_channels; > > + return ret; > > > > - ret = iio_device_register(indio_dev); > > + ret = devm_add_action(&spi->dev, ad7280_sw_power_down, st); > > if (ret) > What state are we left in if the devm_add_action fails? > > Answer: Everything is unwound except the thing we were adding the action > for. So you need to call ad7280_sw_power_down in the error path here. OK. I have also moved the devm_add_action call just after spi_setup in v5. So the action will be also called for fail in: ad7280_chain_setup, ad7280_channel_init and ad7280_attr_init. I think it is ok to do so if we are calling this action also when devm_add_action fails. > > - goto error_free_attr; > > + return ret; Thank you for review! -- Slawomir Stepien
