On Wed, 5 Apr 2023 07:21:00 -0700 Lars-Peter Clausen <lars@xxxxxxxxxx> wrote: > On 4/5/23 07:01, Nicolas Frattaroli wrote: > > [...] > > + state->vdd_reg = devm_regulator_get(&spi->dev, "vdd"); > > + if (IS_ERR(state->vdd_reg)) { > > + ret = dev_err_probe(&spi->dev, PTR_ERR(state->vdd_reg), > > + "vdd regulator not specified\n"); > > + goto error_disable_vref_reg; > > + } > > + ret = regulator_enable(state->vdd_reg); > > + if (ret) { > > + dev_err(&spi->dev, "Failed to enable vdd regulator: %d\n", > > + ret); > > + goto error_disable_vref_reg; > > + } > The two above can be combined into `devm_regulator_get_enable()`. This > will also take care of automatically disabling the regulator on the > error path and on remove. I'm not keen on the ordering of probe wrt to remove that results from mixing devm and not. Note that already happens because of the gets vs enables so another reason to take this driver fully devm_ based. Jonathan > > + > > spi_set_drvdata(spi, indio_dev); > > id = spi_get_device_id(spi); > > indio_dev->info = &mcp4922_info; > > [...] > >