Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

iio: frequency: adf4350: Use device managed functions and fix power down issue.

The devm_clk_get_enabled() helper:
- calls devm_clk_get()
- calls clk_prepare_enable() and registers what is needed in order to
call clk_disable_unprepare() when needed, as a managed resource.

Also replace devm_regulator_get() and regulator_enable() with
devm_regulator_get_enable() helper and remove regulator_disable().

Replace iio_device_register() with devm_iio_device_register() and remove
iio_device_unregister().

And st->reg is not used anymore, so remove it.

As Jonathan pointed out, couple of things that are wrong:

1) The device is powered down 'before' we unregister it with the
subsystem and as such userspace interfaces are still exposed which
probably won't do the right thing if the chip is powered down.

2) This isn't done in the error paths in probe.

To solve this problem, register a new callback adf4350_power_down()
with devm_add_action_or_reset(), to enable software power down in both
error and device detach path. So the remove function can be removed.

Remove spi_set_drvdata() from the probe function, since spi_get_drvdata()
is not used anymore.

Fixes: e31166f0fd48 ("iio: frequency: New driver for Analog Devices ADF4350/ADF4351 Wideband Synthesizers")
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Link: https://lore.kernel.org/r/20230828062717.2310219-1-ruanjinjie@huawei.com
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

authored by

Jinjie Ruan and committed by
Jonathan Cameron
9979cc64 da2737c9

+23 -52
+23 -52
drivers/iio/frequency/adf4350.c
··· 33 33 34 34 struct adf4350_state { 35 35 struct spi_device *spi; 36 - struct regulator *reg; 37 36 struct gpio_desc *lock_detect_gpiod; 38 37 struct adf4350_platform_data *pdata; 39 38 struct clk *clk; ··· 468 469 return pdata; 469 470 } 470 471 472 + static void adf4350_power_down(void *data) 473 + { 474 + struct iio_dev *indio_dev = data; 475 + struct adf4350_state *st = iio_priv(indio_dev); 476 + 477 + st->regs[ADF4350_REG2] |= ADF4350_REG2_POWER_DOWN_EN; 478 + adf4350_sync_config(st); 479 + } 480 + 471 481 static int adf4350_probe(struct spi_device *spi) 472 482 { 473 483 struct adf4350_platform_data *pdata; ··· 499 491 } 500 492 501 493 if (!pdata->clkin) { 502 - clk = devm_clk_get(&spi->dev, "clkin"); 494 + clk = devm_clk_get_enabled(&spi->dev, "clkin"); 503 495 if (IS_ERR(clk)) 504 - return -EPROBE_DEFER; 505 - 506 - ret = clk_prepare_enable(clk); 507 - if (ret < 0) 508 - return ret; 496 + return PTR_ERR(clk); 509 497 } 510 498 511 499 indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); 512 - if (indio_dev == NULL) { 513 - ret = -ENOMEM; 514 - goto error_disable_clk; 515 - } 500 + if (indio_dev == NULL) 501 + return -ENOMEM; 516 502 517 503 st = iio_priv(indio_dev); 518 504 519 - st->reg = devm_regulator_get(&spi->dev, "vcc"); 520 - if (!IS_ERR(st->reg)) { 521 - ret = regulator_enable(st->reg); 522 - if (ret) 523 - goto error_disable_clk; 524 - } 505 + ret = devm_regulator_get_enable(&spi->dev, "vcc"); 506 + if (ret) 507 + return ret; 525 508 526 - spi_set_drvdata(spi, indio_dev); 527 509 st->spi = spi; 528 510 st->pdata = pdata; 529 511 ··· 542 544 543 545 st->lock_detect_gpiod = devm_gpiod_get_optional(&spi->dev, NULL, 544 546 GPIOD_IN); 545 - if (IS_ERR(st->lock_detect_gpiod)) { 546 - ret = PTR_ERR(st->lock_detect_gpiod); 547 - goto error_disable_reg; 548 - } 547 + if (IS_ERR(st->lock_detect_gpiod)) 548 + return PTR_ERR(st->lock_detect_gpiod); 549 549 550 550 if (pdata->power_up_frequency) { 551 551 ret = adf4350_set_freq(st, pdata->power_up_frequency); 552 552 if (ret) 553 - goto error_disable_reg; 553 + return ret; 554 554 } 555 555 556 - ret = iio_device_register(indio_dev); 556 + ret = devm_add_action_or_reset(&spi->dev, adf4350_power_down, indio_dev); 557 557 if (ret) 558 - goto error_disable_reg; 558 + return dev_err_probe(&spi->dev, ret, 559 + "Failed to add action to managed power down\n"); 559 560 560 - return 0; 561 - 562 - error_disable_reg: 563 - if (!IS_ERR(st->reg)) 564 - regulator_disable(st->reg); 565 - error_disable_clk: 566 - clk_disable_unprepare(clk); 567 - 568 - return ret; 569 - } 570 - 571 - static void adf4350_remove(struct spi_device *spi) 572 - { 573 - struct iio_dev *indio_dev = spi_get_drvdata(spi); 574 - struct adf4350_state *st = iio_priv(indio_dev); 575 - struct regulator *reg = st->reg; 576 - 577 - st->regs[ADF4350_REG2] |= ADF4350_REG2_POWER_DOWN_EN; 578 - adf4350_sync_config(st); 579 - 580 - iio_device_unregister(indio_dev); 581 - 582 - clk_disable_unprepare(st->clk); 583 - 584 - if (!IS_ERR(reg)) 585 - regulator_disable(reg); 561 + return devm_iio_device_register(&spi->dev, indio_dev); 586 562 } 587 563 588 564 static const struct of_device_id adf4350_of_match[] = { ··· 579 607 .of_match_table = adf4350_of_match, 580 608 }, 581 609 .probe = adf4350_probe, 582 - .remove = adf4350_remove, 583 610 .id_table = adf4350_id, 584 611 }; 585 612 module_spi_driver(adf4350_driver);