* [PATCH 01/13] iio: adc: ad7124: Benefit from devm_clk_get_enabled() to simplify
@ 2022-08-08 20:47 Uwe Kleine-König
2022-08-08 20:47 ` [PATCH 02/13] iio: adc: ad7768-1: " Uwe Kleine-König
` (12 more replies)
0 siblings, 13 replies; 30+ messages in thread
From: Uwe Kleine-König @ 2022-08-08 20:47 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, kernel,
Jonathan Cameron, Nuno Sa, Michal Simek
Make use of devm_clk_get_enabled() to replace some code that effectively
open codes this new function.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/adc/ad7124.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
index c5b785d8b241..4088786e1026 100644
--- a/drivers/iio/adc/ad7124.c
+++ b/drivers/iio/adc/ad7124.c
@@ -936,11 +936,6 @@ static void ad7124_reg_disable(void *r)
regulator_disable(r);
}
-static void ad7124_clk_disable(void *c)
-{
- clk_disable_unprepare(c);
-}
-
static int ad7124_probe(struct spi_device *spi)
{
const struct ad7124_chip_info *info;
@@ -993,18 +988,10 @@ static int ad7124_probe(struct spi_device *spi)
return ret;
}
- st->mclk = devm_clk_get(&spi->dev, "mclk");
+ st->mclk = devm_clk_get_enabled(&spi->dev, "mclk");
if (IS_ERR(st->mclk))
return PTR_ERR(st->mclk);
- ret = clk_prepare_enable(st->mclk);
- if (ret < 0)
- return ret;
-
- ret = devm_add_action_or_reset(&spi->dev, ad7124_clk_disable, st->mclk);
- if (ret)
- return ret;
-
ret = ad7124_soft_reset(st);
if (ret < 0)
return ret;
base-commit: 8b3d743fc9e2542822826890b482afabf0e7522a
--
2.36.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 02/13] iio: adc: ad7768-1: Benefit from devm_clk_get_enabled() to simplify
2022-08-08 20:47 [PATCH 01/13] iio: adc: ad7124: Benefit from devm_clk_get_enabled() to simplify Uwe Kleine-König
@ 2022-08-08 20:47 ` Uwe Kleine-König
2022-08-08 20:47 ` [PATCH 03/13] iio: adc: ad9467: " Uwe Kleine-König
` (11 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Uwe Kleine-König @ 2022-08-08 20:47 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, kernel,
Jonathan Cameron, Nuno Sa, Michal Simek
Make use of devm_clk_get_enabled() to replace some code that effectively
open codes this new function.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/adc/ad7768-1.c | 17 +----------------
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index aa42ba759fa1..8ae34ed8fabd 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -539,13 +539,6 @@ static void ad7768_regulator_disable(void *data)
regulator_disable(st->vref);
}
-static void ad7768_clk_disable(void *data)
-{
- struct ad7768_state *st = data;
-
- clk_disable_unprepare(st->mclk);
-}
-
static int ad7768_set_channel_label(struct iio_dev *indio_dev,
int num_channels)
{
@@ -600,18 +593,10 @@ static int ad7768_probe(struct spi_device *spi)
if (ret)
return ret;
- st->mclk = devm_clk_get(&spi->dev, "mclk");
+ st->mclk = devm_clk_get_enabled(&spi->dev, "mclk");
if (IS_ERR(st->mclk))
return PTR_ERR(st->mclk);
- ret = clk_prepare_enable(st->mclk);
- if (ret < 0)
- return ret;
-
- ret = devm_add_action_or_reset(&spi->dev, ad7768_clk_disable, st);
- if (ret)
- return ret;
-
st->mclk_freq = clk_get_rate(st->mclk);
mutex_init(&st->lock);
--
2.36.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 03/13] iio: adc: ad9467: Benefit from devm_clk_get_enabled() to simplify
2022-08-08 20:47 [PATCH 01/13] iio: adc: ad7124: Benefit from devm_clk_get_enabled() to simplify Uwe Kleine-König
2022-08-08 20:47 ` [PATCH 02/13] iio: adc: ad7768-1: " Uwe Kleine-König
@ 2022-08-08 20:47 ` Uwe Kleine-König
2022-08-08 20:47 ` [PATCH 04/13] iio: adc: ad9467: Benefit from devm_clk_get_prepared() " Uwe Kleine-König
` (10 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Uwe Kleine-König @ 2022-08-08 20:47 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, kernel,
Jonathan Cameron, Nuno Sa, Michal Simek
Make use of devm_clk_get_enabled() to replace some code that effectively
open codes this new function.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/adc/ad9467.c | 17 +----------------
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
index dbfc8517cb8a..a07df0fd3329 100644
--- a/drivers/iio/adc/ad9467.c
+++ b/drivers/iio/adc/ad9467.c
@@ -378,13 +378,6 @@ static int ad9467_preenable_setup(struct adi_axi_adc_conv *conv)
return ad9467_outputmode_set(st->spi, st->output_mode);
}
-static void ad9467_clk_disable(void *data)
-{
- struct ad9467_state *st = data;
-
- clk_disable_unprepare(st->clk);
-}
-
static int ad9467_probe(struct spi_device *spi)
{
const struct ad9467_chip_info *info;
@@ -404,18 +397,10 @@ static int ad9467_probe(struct spi_device *spi)
st = adi_axi_adc_conv_priv(conv);
st->spi = spi;
- st->clk = devm_clk_get(&spi->dev, "adc-clk");
+ st->clk = devm_clk_get_enabled(&spi->dev, "adc-clk");
if (IS_ERR(st->clk))
return PTR_ERR(st->clk);
- ret = clk_prepare_enable(st->clk);
- if (ret < 0)
- return ret;
-
- ret = devm_add_action_or_reset(&spi->dev, ad9467_clk_disable, st);
- if (ret)
- return ret;
-
st->pwrdown_gpio = devm_gpiod_get_optional(&spi->dev, "powerdown",
GPIOD_OUT_LOW);
if (IS_ERR(st->pwrdown_gpio))
--
2.36.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 04/13] iio: adc: ad9467: Benefit from devm_clk_get_prepared() to simplify
2022-08-08 20:47 [PATCH 01/13] iio: adc: ad7124: Benefit from devm_clk_get_enabled() to simplify Uwe Kleine-König
2022-08-08 20:47 ` [PATCH 02/13] iio: adc: ad7768-1: " Uwe Kleine-König
2022-08-08 20:47 ` [PATCH 03/13] iio: adc: ad9467: " Uwe Kleine-König
@ 2022-08-08 20:47 ` Uwe Kleine-König
2022-08-08 21:01 ` Andy Shevchenko
2022-08-08 20:47 ` [PATCH 05/13] iio: adc: lpc18xx: Benefit from devm_clk_get_enabled() " Uwe Kleine-König
` (9 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Uwe Kleine-König @ 2022-08-08 20:47 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, kernel,
Jonathan Cameron, Nuno Sa, Michal Simek
Make use of devm_clk_get_prepared() to replace some code that effectively
open codes this new function.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/adc/ingenic-adc.c | 15 ++-------------
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/iio/adc/ingenic-adc.c b/drivers/iio/adc/ingenic-adc.c
index 2b3912c6ca6b..b6433bcd53f5 100644
--- a/drivers/iio/adc/ingenic-adc.c
+++ b/drivers/iio/adc/ingenic-adc.c
@@ -732,11 +732,6 @@ static int ingenic_adc_of_xlate(struct iio_dev *iio_dev,
return -EINVAL;
}
-static void ingenic_adc_clk_cleanup(void *data)
-{
- clk_unprepare(data);
-}
-
static const struct iio_info ingenic_adc_info = {
.write_raw = ingenic_adc_write_raw,
.read_raw = ingenic_adc_read_raw,
@@ -856,13 +851,13 @@ static int ingenic_adc_probe(struct platform_device *pdev)
if (IS_ERR(adc->base))
return PTR_ERR(adc->base);
- adc->clk = devm_clk_get(dev, "adc");
+ adc->clk = devm_clk_get_prepared(dev, "adc");
if (IS_ERR(adc->clk)) {
dev_err(dev, "Unable to get clock\n");
return PTR_ERR(adc->clk);
}
- ret = clk_prepare_enable(adc->clk);
+ ret = clk_enable(adc->clk);
if (ret) {
dev_err(dev, "Failed to enable clock\n");
return ret;
@@ -891,12 +886,6 @@ static int ingenic_adc_probe(struct platform_device *pdev)
usleep_range(2000, 3000); /* Must wait at least 2ms. */
clk_disable(adc->clk);
- ret = devm_add_action_or_reset(dev, ingenic_adc_clk_cleanup, adc->clk);
- if (ret) {
- dev_err(dev, "Unable to add action\n");
- return ret;
- }
-
iio_dev->name = "jz-adc";
iio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
iio_dev->setup_ops = &ingenic_buffer_setup_ops;
--
2.36.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 05/13] iio: adc: lpc18xx: Benefit from devm_clk_get_enabled() to simplify
2022-08-08 20:47 [PATCH 01/13] iio: adc: ad7124: Benefit from devm_clk_get_enabled() to simplify Uwe Kleine-König
` (2 preceding siblings ...)
2022-08-08 20:47 ` [PATCH 04/13] iio: adc: ad9467: Benefit from devm_clk_get_prepared() " Uwe Kleine-König
@ 2022-08-08 20:47 ` Uwe Kleine-König
2022-08-13 16:25 ` Jonathan Cameron
2022-08-08 20:47 ` [PATCH 06/13] iio: adc: rockchip_saradc: " Uwe Kleine-König
` (8 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Uwe Kleine-König @ 2022-08-08 20:47 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, kernel,
Jonathan Cameron, Nuno Sa, Michal Simek
Make use of devm_clk_get_enabled() to replace some code that effectively
open codes this new function.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/adc/lpc18xx_adc.c | 18 +-----------------
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/drivers/iio/adc/lpc18xx_adc.c b/drivers/iio/adc/lpc18xx_adc.c
index ae9c9384f23e..8bbc18aaa474 100644
--- a/drivers/iio/adc/lpc18xx_adc.c
+++ b/drivers/iio/adc/lpc18xx_adc.c
@@ -122,11 +122,6 @@ static void lpc18xx_clear_cr_reg(void *data)
writel(0, adc->base + LPC18XX_ADC_CR);
}
-static void lpc18xx_clk_disable(void *clk)
-{
- clk_disable_unprepare(clk);
-}
-
static void lpc18xx_regulator_disable(void *vref)
{
regulator_disable(vref);
@@ -152,7 +147,7 @@ static int lpc18xx_adc_probe(struct platform_device *pdev)
if (IS_ERR(adc->base))
return PTR_ERR(adc->base);
- adc->clk = devm_clk_get(&pdev->dev, NULL);
+ adc->clk = devm_clk_get_enabled(&pdev->dev, NULL);
if (IS_ERR(adc->clk))
return dev_err_probe(&pdev->dev, PTR_ERR(adc->clk),
"error getting clock\n");
@@ -178,17 +173,6 @@ static int lpc18xx_adc_probe(struct platform_device *pdev)
if (ret)
return ret;
- ret = clk_prepare_enable(adc->clk);
- if (ret) {
- dev_err(&pdev->dev, "unable to enable clock\n");
- return ret;
- }
-
- ret = devm_add_action_or_reset(&pdev->dev, lpc18xx_clk_disable,
- adc->clk);
- if (ret)
- return ret;
-
rate = clk_get_rate(adc->clk);
clkdiv = DIV_ROUND_UP(rate, LPC18XX_ADC_CLK_TARGET);
--
2.36.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 06/13] iio: adc: rockchip_saradc: Benefit from devm_clk_get_enabled() to simplify
2022-08-08 20:47 [PATCH 01/13] iio: adc: ad7124: Benefit from devm_clk_get_enabled() to simplify Uwe Kleine-König
` (3 preceding siblings ...)
2022-08-08 20:47 ` [PATCH 05/13] iio: adc: lpc18xx: Benefit from devm_clk_get_enabled() " Uwe Kleine-König
@ 2022-08-08 20:47 ` Uwe Kleine-König
2022-08-13 16:31 ` Jonathan Cameron
2022-08-08 20:47 ` [PATCH 07/13] iio: adc: ti-ads131e08: " Uwe Kleine-König
` (7 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Uwe Kleine-König @ 2022-08-08 20:47 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, kernel,
Jonathan Cameron, Nuno Sa, Michal Simek
Make use of devm_clk_get_enabled() to replace some code that effectively
open codes this new function.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/adc/rockchip_saradc.c | 44 ++-----------------------------
1 file changed, 2 insertions(+), 42 deletions(-)
diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
index b87ea7148b58..db1190faa223 100644
--- a/drivers/iio/adc/rockchip_saradc.c
+++ b/drivers/iio/adc/rockchip_saradc.c
@@ -233,20 +233,6 @@ static void rockchip_saradc_reset_controller(struct reset_control *reset)
reset_control_deassert(reset);
}
-static void rockchip_saradc_clk_disable(void *data)
-{
- struct rockchip_saradc *info = data;
-
- clk_disable_unprepare(info->clk);
-}
-
-static void rockchip_saradc_pclk_disable(void *data)
-{
- struct rockchip_saradc *info = data;
-
- clk_disable_unprepare(info->pclk);
-}
-
static void rockchip_saradc_regulator_disable(void *data)
{
struct rockchip_saradc *info = data;
@@ -380,12 +366,12 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
return ret;
}
- info->pclk = devm_clk_get(&pdev->dev, "apb_pclk");
+ info->pclk = devm_clk_get_enabled(&pdev->dev, "apb_pclk");
if (IS_ERR(info->pclk))
return dev_err_probe(&pdev->dev, PTR_ERR(info->pclk),
"failed to get pclk\n");
- info->clk = devm_clk_get(&pdev->dev, "saradc");
+ info->clk = devm_clk_get_enabled(&pdev->dev, "saradc");
if (IS_ERR(info->clk))
return dev_err_probe(&pdev->dev, PTR_ERR(info->clk),
"failed to get adc clock\n");
@@ -427,32 +413,6 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
info->uv_vref = ret;
- ret = clk_prepare_enable(info->pclk);
- if (ret < 0) {
- dev_err(&pdev->dev, "failed to enable pclk\n");
- return ret;
- }
- ret = devm_add_action_or_reset(&pdev->dev,
- rockchip_saradc_pclk_disable, info);
- if (ret) {
- dev_err(&pdev->dev, "failed to register devm action, %d\n",
- ret);
- return ret;
- }
-
- ret = clk_prepare_enable(info->clk);
- if (ret < 0) {
- dev_err(&pdev->dev, "failed to enable converter clock\n");
- return ret;
- }
- ret = devm_add_action_or_reset(&pdev->dev,
- rockchip_saradc_clk_disable, info);
- if (ret) {
- dev_err(&pdev->dev, "failed to register devm action, %d\n",
- ret);
- return ret;
- }
-
platform_set_drvdata(pdev, indio_dev);
indio_dev->name = dev_name(&pdev->dev);
--
2.36.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 07/13] iio: adc: ti-ads131e08: Benefit from devm_clk_get_enabled() to simplify
2022-08-08 20:47 [PATCH 01/13] iio: adc: ad7124: Benefit from devm_clk_get_enabled() to simplify Uwe Kleine-König
` (4 preceding siblings ...)
2022-08-08 20:47 ` [PATCH 06/13] iio: adc: rockchip_saradc: " Uwe Kleine-König
@ 2022-08-08 20:47 ` Uwe Kleine-König
2022-08-08 20:47 ` [PATCH 08/13] iio: adc: xilinx-ams: " Uwe Kleine-König
` (6 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Uwe Kleine-König @ 2022-08-08 20:47 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, kernel,
Jonathan Cameron, Nuno Sa, Michal Simek
Make use of devm_clk_get_enabled() to replace some code that effectively
open codes this new function.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/adc/ti-ads131e08.c | 19 +------------------
1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/drivers/iio/adc/ti-ads131e08.c b/drivers/iio/adc/ti-ads131e08.c
index 0c2025a22575..7d7a8f0d3ab5 100644
--- a/drivers/iio/adc/ti-ads131e08.c
+++ b/drivers/iio/adc/ti-ads131e08.c
@@ -793,13 +793,6 @@ static void ads131e08_regulator_disable(void *data)
regulator_disable(st->vref_reg);
}
-static void ads131e08_clk_disable(void *data)
-{
- struct ads131e08_state *st = data;
-
- clk_disable_unprepare(st->adc_clk);
-}
-
static int ads131e08_probe(struct spi_device *spi)
{
const struct ads131e08_info *info;
@@ -892,21 +885,11 @@ static int ads131e08_probe(struct spi_device *spi)
st->vref_reg = NULL;
}
- st->adc_clk = devm_clk_get(&spi->dev, "adc-clk");
+ st->adc_clk = devm_clk_get_enabled(&spi->dev, "adc-clk");
if (IS_ERR(st->adc_clk))
return dev_err_probe(&spi->dev, PTR_ERR(st->adc_clk),
"failed to get the ADC clock\n");
- ret = clk_prepare_enable(st->adc_clk);
- if (ret) {
- dev_err(&spi->dev, "failed to prepare/enable the ADC clock\n");
- return ret;
- }
-
- ret = devm_add_action_or_reset(&spi->dev, ads131e08_clk_disable, st);
- if (ret)
- return ret;
-
adc_clk_hz = clk_get_rate(st->adc_clk);
if (!adc_clk_hz) {
dev_err(&spi->dev, "failed to get the ADC clock rate\n");
--
2.36.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 08/13] iio: adc: xilinx-ams: Benefit from devm_clk_get_enabled() to simplify
2022-08-08 20:47 [PATCH 01/13] iio: adc: ad7124: Benefit from devm_clk_get_enabled() to simplify Uwe Kleine-König
` (5 preceding siblings ...)
2022-08-08 20:47 ` [PATCH 07/13] iio: adc: ti-ads131e08: " Uwe Kleine-König
@ 2022-08-08 20:47 ` Uwe Kleine-König
2022-08-08 20:47 ` [PATCH 09/13] iio: adc: xilinx-xadc: " Uwe Kleine-König
` (5 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Uwe Kleine-König @ 2022-08-08 20:47 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, kernel,
Jonathan Cameron, Nuno Sa, Michal Simek
Make use of devm_clk_get_enabled() to replace some code that effectively
open codes this new function.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/adc/xilinx-ams.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/iio/adc/xilinx-ams.c b/drivers/iio/adc/xilinx-ams.c
index a55396c1f8b2..2cab7f0c0633 100644
--- a/drivers/iio/adc/xilinx-ams.c
+++ b/drivers/iio/adc/xilinx-ams.c
@@ -1351,11 +1351,6 @@ static const struct of_device_id ams_of_match_table[] = {
};
MODULE_DEVICE_TABLE(of, ams_of_match_table);
-static void ams_clk_disable_unprepare(void *data)
-{
- clk_disable_unprepare(data);
-}
-
static int ams_probe(struct platform_device *pdev)
{
struct iio_dev *indio_dev;
@@ -1380,18 +1375,10 @@ static int ams_probe(struct platform_device *pdev)
if (IS_ERR(ams->base))
return PTR_ERR(ams->base);
- ams->clk = devm_clk_get(&pdev->dev, NULL);
+ ams->clk = devm_clk_get_enabled(&pdev->dev, NULL);
if (IS_ERR(ams->clk))
return PTR_ERR(ams->clk);
- ret = clk_prepare_enable(ams->clk);
- if (ret < 0)
- return ret;
-
- ret = devm_add_action_or_reset(&pdev->dev, ams_clk_disable_unprepare, ams->clk);
- if (ret < 0)
- return ret;
-
ret = devm_delayed_work_autocancel(&pdev->dev, &ams->ams_unmask_work,
ams_unmask_worker);
if (ret < 0)
--
2.36.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 09/13] iio: adc: xilinx-xadc: Benefit from devm_clk_get_enabled() to simplify
2022-08-08 20:47 [PATCH 01/13] iio: adc: ad7124: Benefit from devm_clk_get_enabled() to simplify Uwe Kleine-König
` (6 preceding siblings ...)
2022-08-08 20:47 ` [PATCH 08/13] iio: adc: xilinx-ams: " Uwe Kleine-König
@ 2022-08-08 20:47 ` Uwe Kleine-König
2022-08-08 20:47 ` [PATCH 10/13] iio: frequency: adf4371: " Uwe Kleine-König
` (4 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Uwe Kleine-König @ 2022-08-08 20:47 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, kernel,
Jonathan Cameron, Nuno Sa, Michal Simek
Make use of devm_clk_get_enabled() to replace some code that effectively
open codes this new function.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/adc/xilinx-xadc-core.c | 18 +-----------------
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index 823c8e5f9809..a520e07e4e08 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -1299,13 +1299,6 @@ static const char * const xadc_type_names[] = {
[XADC_TYPE_US] = "xilinx-system-monitor",
};
-static void xadc_clk_disable_unprepare(void *data)
-{
- struct clk *clk = data;
-
- clk_disable_unprepare(clk);
-}
-
static void xadc_cancel_delayed_work(void *data)
{
struct delayed_work *work = data;
@@ -1383,19 +1376,10 @@ static int xadc_probe(struct platform_device *pdev)
}
}
- xadc->clk = devm_clk_get(dev, NULL);
+ xadc->clk = devm_clk_get_enabled(dev, NULL);
if (IS_ERR(xadc->clk))
return PTR_ERR(xadc->clk);
- ret = clk_prepare_enable(xadc->clk);
- if (ret)
- return ret;
-
- ret = devm_add_action_or_reset(dev,
- xadc_clk_disable_unprepare, xadc->clk);
- if (ret)
- return ret;
-
/*
* Make sure not to exceed the maximum samplerate since otherwise the
* resulting interrupt storm will soft-lock the system.
--
2.36.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 10/13] iio: frequency: adf4371: Benefit from devm_clk_get_enabled() to simplify
2022-08-08 20:47 [PATCH 01/13] iio: adc: ad7124: Benefit from devm_clk_get_enabled() to simplify Uwe Kleine-König
` (7 preceding siblings ...)
2022-08-08 20:47 ` [PATCH 09/13] iio: adc: xilinx-xadc: " Uwe Kleine-König
@ 2022-08-08 20:47 ` Uwe Kleine-König
2022-08-08 20:47 ` [PATCH 11/13] iio: frequency: admv1013: " Uwe Kleine-König
` (3 subsequent siblings)
12 siblings, 0 replies; 30+ messages in thread
From: Uwe Kleine-König @ 2022-08-08 20:47 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, kernel,
Jonathan Cameron, Nuno Sa, Michal Simek
Make use of devm_clk_get_enabled() to replace some code that effectively
open codes this new function.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/frequency/adf4371.c | 17 +----------------
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/drivers/iio/frequency/adf4371.c b/drivers/iio/frequency/adf4371.c
index ecd5e18995ad..54040b5fded0 100644
--- a/drivers/iio/frequency/adf4371.c
+++ b/drivers/iio/frequency/adf4371.c
@@ -540,13 +540,6 @@ static int adf4371_setup(struct adf4371_state *st)
return regmap_bulk_write(st->regmap, ADF4371_REG(0x30), st->buf, 5);
}
-static void adf4371_clk_disable(void *data)
-{
- struct adf4371_state *st = data;
-
- clk_disable_unprepare(st->clkin);
-}
-
static int adf4371_probe(struct spi_device *spi)
{
const struct spi_device_id *id = spi_get_device_id(spi);
@@ -579,18 +572,10 @@ static int adf4371_probe(struct spi_device *spi)
indio_dev->channels = st->chip_info->channels;
indio_dev->num_channels = st->chip_info->num_channels;
- st->clkin = devm_clk_get(&spi->dev, "clkin");
+ st->clkin = devm_clk_get_enabled(&spi->dev, "clkin");
if (IS_ERR(st->clkin))
return PTR_ERR(st->clkin);
- ret = clk_prepare_enable(st->clkin);
- if (ret < 0)
- return ret;
-
- ret = devm_add_action_or_reset(&spi->dev, adf4371_clk_disable, st);
- if (ret)
- return ret;
-
st->clkin_freq = clk_get_rate(st->clkin);
ret = adf4371_setup(st);
--
2.36.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 11/13] iio: frequency: admv1013: Benefit from devm_clk_get_enabled() to simplify
2022-08-08 20:47 [PATCH 01/13] iio: adc: ad7124: Benefit from devm_clk_get_enabled() to simplify Uwe Kleine-König
` (8 preceding siblings ...)
2022-08-08 20:47 ` [PATCH 10/13] iio: frequency: adf4371: " Uwe Kleine-König
@ 2022-08-08 20:47 ` Uwe Kleine-König
2022-08-13 16:37 ` Jonathan Cameron
2022-08-08 20:47 ` [PATCH 12/13] iio: frequency: adrf6780: " Uwe Kleine-König
` (2 subsequent siblings)
12 siblings, 1 reply; 30+ messages in thread
From: Uwe Kleine-König @ 2022-08-08 20:47 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, kernel,
Jonathan Cameron, Nuno Sa, Michal Simek
Make use of devm_clk_get_enabled() to replace some code that effectively
open codes this new function.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/frequency/admv1013.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/iio/frequency/admv1013.c b/drivers/iio/frequency/admv1013.c
index b0e1f6571afb..1346d77df77f 100644
--- a/drivers/iio/frequency/admv1013.c
+++ b/drivers/iio/frequency/admv1013.c
@@ -490,11 +490,6 @@ static int admv1013_init(struct admv1013_state *st)
st->input_mode);
}
-static void admv1013_clk_disable(void *data)
-{
- clk_disable_unprepare(data);
-}
-
static void admv1013_reg_disable(void *data)
{
regulator_disable(data);
@@ -559,7 +554,7 @@ static int admv1013_properties_parse(struct admv1013_state *st)
return dev_err_probe(&spi->dev, PTR_ERR(st->reg),
"failed to get the common-mode voltage\n");
- st->clkin = devm_clk_get(&spi->dev, "lo_in");
+ st->clkin = devm_clk_get_enabled(&spi->dev, "lo_in");
if (IS_ERR(st->clkin))
return dev_err_probe(&spi->dev, PTR_ERR(st->clkin),
"failed to get the LO input clock\n");
@@ -601,14 +596,6 @@ static int admv1013_probe(struct spi_device *spi)
if (ret)
return ret;
- ret = clk_prepare_enable(st->clkin);
- if (ret)
- return ret;
-
- ret = devm_add_action_or_reset(&spi->dev, admv1013_clk_disable, st->clkin);
- if (ret)
- return ret;
-
st->nb.notifier_call = admv1013_freq_change;
ret = devm_clk_notifier_register(&spi->dev, st->clkin, &st->nb);
if (ret)
--
2.36.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 12/13] iio: frequency: adrf6780: Benefit from devm_clk_get_enabled() to simplify
2022-08-08 20:47 [PATCH 01/13] iio: adc: ad7124: Benefit from devm_clk_get_enabled() to simplify Uwe Kleine-König
` (9 preceding siblings ...)
2022-08-08 20:47 ` [PATCH 11/13] iio: frequency: admv1013: " Uwe Kleine-König
@ 2022-08-08 20:47 ` Uwe Kleine-König
2022-08-08 20:47 ` [PATCH 13/13] iio: imu: adis16475: " Uwe Kleine-König
2022-08-13 16:39 ` [PATCH 01/13] iio: adc: ad7124: " Jonathan Cameron
12 siblings, 0 replies; 30+ messages in thread
From: Uwe Kleine-König @ 2022-08-08 20:47 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, kernel,
Jonathan Cameron, Nuno Sa, Michal Simek
Make use of devm_clk_get_enabled() to replace some code that effectively
open codes this new function.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/frequency/adrf6780.c | 16 +---------------
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/drivers/iio/frequency/adrf6780.c b/drivers/iio/frequency/adrf6780.c
index 8255ffd174f6..d7bf34ceedd8 100644
--- a/drivers/iio/frequency/adrf6780.c
+++ b/drivers/iio/frequency/adrf6780.c
@@ -441,11 +441,6 @@ static void adrf6780_properties_parse(struct adrf6780_state *st)
st->vdet_out_en = device_property_read_bool(&spi->dev, "adi,vdet-out-en");
}
-static void adrf6780_clk_disable(void *data)
-{
- clk_disable_unprepare(data);
-}
-
static void adrf6780_powerdown(void *data)
{
/* Disable all components in the Enable Register */
@@ -473,20 +468,11 @@ static int adrf6780_probe(struct spi_device *spi)
adrf6780_properties_parse(st);
- st->clkin = devm_clk_get(&spi->dev, "lo_in");
+ st->clkin = devm_clk_get_enabled(&spi->dev, "lo_in");
if (IS_ERR(st->clkin))
return dev_err_probe(&spi->dev, PTR_ERR(st->clkin),
"failed to get the LO input clock\n");
- ret = clk_prepare_enable(st->clkin);
- if (ret)
- return ret;
-
- ret = devm_add_action_or_reset(&spi->dev, adrf6780_clk_disable,
- st->clkin);
- if (ret)
- return ret;
-
mutex_init(&st->lock);
ret = adrf6780_init(st);
--
2.36.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH 13/13] iio: imu: adis16475: Benefit from devm_clk_get_enabled() to simplify
2022-08-08 20:47 [PATCH 01/13] iio: adc: ad7124: Benefit from devm_clk_get_enabled() to simplify Uwe Kleine-König
` (10 preceding siblings ...)
2022-08-08 20:47 ` [PATCH 12/13] iio: frequency: adrf6780: " Uwe Kleine-König
@ 2022-08-08 20:47 ` Uwe Kleine-König
2022-08-13 16:39 ` [PATCH 01/13] iio: adc: ad7124: " Jonathan Cameron
12 siblings, 0 replies; 30+ messages in thread
From: Uwe Kleine-König @ 2022-08-08 20:47 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, kernel,
Jonathan Cameron, Nuno Sa, Michal Simek
Make use of devm_clk_get_enabled() to replace some code that effectively
open codes this new function.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/iio/imu/adis16475.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/drivers/iio/imu/adis16475.c b/drivers/iio/imu/adis16475.c
index ff2b0fab840a..aec55f7e1f26 100644
--- a/drivers/iio/imu/adis16475.c
+++ b/drivers/iio/imu/adis16475.c
@@ -1120,11 +1120,6 @@ static irqreturn_t adis16475_trigger_handler(int irq, void *p)
return IRQ_HANDLED;
}
-static void adis16475_disable_clk(void *data)
-{
- clk_disable_unprepare((struct clk *)data);
-}
-
static int adis16475_config_sync_mode(struct adis16475 *st)
{
int ret;
@@ -1150,19 +1145,11 @@ static int adis16475_config_sync_mode(struct adis16475 *st)
/* All the other modes require external input signal */
if (sync->sync_mode != ADIS16475_SYNC_OUTPUT) {
- struct clk *clk = devm_clk_get(dev, NULL);
+ struct clk *clk = devm_clk_get_enabled(dev, NULL);
if (IS_ERR(clk))
return PTR_ERR(clk);
- ret = clk_prepare_enable(clk);
- if (ret)
- return ret;
-
- ret = devm_add_action_or_reset(dev, adis16475_disable_clk, clk);
- if (ret)
- return ret;
-
st->clk_freq = clk_get_rate(clk);
if (st->clk_freq < sync->min_rate ||
st->clk_freq > sync->max_rate) {
--
2.36.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 04/13] iio: adc: ad9467: Benefit from devm_clk_get_prepared() to simplify
2022-08-08 20:47 ` [PATCH 04/13] iio: adc: ad9467: Benefit from devm_clk_get_prepared() " Uwe Kleine-König
@ 2022-08-08 21:01 ` Andy Shevchenko
2022-08-09 5:37 ` [PATCH 04/13] iio: adc: ingenic-adc: " Uwe Kleine-König
0 siblings, 1 reply; 30+ messages in thread
From: Andy Shevchenko @ 2022-08-08 21:01 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jonathan Cameron, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, Sascha Hauer,
Jonathan Cameron, Nuno Sa, Michal Simek
On Mon, Aug 8, 2022 at 10:47 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> Make use of devm_clk_get_prepared() to replace some code that effectively
> open codes this new function.
Copy'n'paste typo in the Subject.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 04/13] iio: adc: ingenic-adc: Benefit from devm_clk_get_prepared() to simplify
2022-08-08 21:01 ` Andy Shevchenko
@ 2022-08-09 5:37 ` Uwe Kleine-König
2022-08-13 16:19 ` Jonathan Cameron
0 siblings, 1 reply; 30+ messages in thread
From: Uwe Kleine-König @ 2022-08-09 5:37 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Tomislav Denis, Lars-Peter Clausen, Heiko Stuebner,
Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, Sascha Hauer,
Jonathan Cameron, Nuno Sa, Michal Simek, Jonathan Cameron
[-- Attachment #1: Type: text/plain, Size: 895 bytes --]
Hello Andy,
On Mon, Aug 08, 2022 at 11:01:39PM +0200, Andy Shevchenko wrote:
> On Mon, Aug 8, 2022 at 10:47 PM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> >
> > Make use of devm_clk_get_prepared() to replace some code that effectively
> > open codes this new function.
>
> Copy'n'paste typo in the Subject.
Good catch, thanks. I fixed it in my private tree, so it will be fixed
on resend. The Subject is now:
iio: adc: ingenic-adc: Benefit from devm_clk_get_prepared() to simplify
Will wait a while for that though, maybe Jonathan will take
care when applying without me resending. I also adapted the subject of
this reply, I wonder if that does more good or more harm :-)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 04/13] iio: adc: ingenic-adc: Benefit from devm_clk_get_prepared() to simplify
2022-08-09 5:37 ` [PATCH 04/13] iio: adc: ingenic-adc: " Uwe Kleine-König
@ 2022-08-13 16:19 ` Jonathan Cameron
0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2022-08-13 16:19 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Andy Shevchenko, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, Sascha Hauer,
Jonathan Cameron, Nuno Sa, Michal Simek
On Tue, 9 Aug 2022 07:37:40 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> Hello Andy,
>
> On Mon, Aug 08, 2022 at 11:01:39PM +0200, Andy Shevchenko wrote:
> > On Mon, Aug 8, 2022 at 10:47 PM Uwe Kleine-K_nig
> > <u.kleine-koenig@pengutronix.de> wrote:
> > >
> > > Make use of devm_clk_get_prepared() to replace some code that effectively
> > > open codes this new function.
> >
> > Copy'n'paste typo in the Subject.
>
> Good catch, thanks. I fixed it in my private tree, so it will be fixed
> on resend. The Subject is now:
>
> iio: adc: ingenic-adc: Benefit from devm_clk_get_prepared() to simplify
>
> Will wait a while for that though, maybe Jonathan will take
> care when applying without me resending. I also adapted the subject of
> this reply, I wonder if that does more good or more harm :-)
I tweaked it whilst applying to be as above.
Thanks,
Jonathan
>
> Best regards
> Uwe
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 05/13] iio: adc: lpc18xx: Benefit from devm_clk_get_enabled() to simplify
2022-08-08 20:47 ` [PATCH 05/13] iio: adc: lpc18xx: Benefit from devm_clk_get_enabled() " Uwe Kleine-König
@ 2022-08-13 16:25 ` Jonathan Cameron
0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2022-08-13 16:25 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Andy Shevchenko, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, kernel,
Jonathan Cameron, Nuno Sa, Michal Simek
On Mon, 8 Aug 2022 22:47:32 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> Make use of devm_clk_get_enabled() to replace some code that effectively
> open codes this new function.
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
This makes me very slightly nervous because it juggles the order of
clock enabling and powering up the device (potentially anyway).
It's always possible that will cause problems - though I think it
is unlikely (and would cause all sorts of problems if someone
used a fixed regulator or clock) to I'm applying this anyway.
> ---
> drivers/iio/adc/lpc18xx_adc.c | 18 +-----------------
> 1 file changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/drivers/iio/adc/lpc18xx_adc.c b/drivers/iio/adc/lpc18xx_adc.c
> index ae9c9384f23e..8bbc18aaa474 100644
> --- a/drivers/iio/adc/lpc18xx_adc.c
> +++ b/drivers/iio/adc/lpc18xx_adc.c
> @@ -122,11 +122,6 @@ static void lpc18xx_clear_cr_reg(void *data)
> writel(0, adc->base + LPC18XX_ADC_CR);
> }
>
> -static void lpc18xx_clk_disable(void *clk)
> -{
> - clk_disable_unprepare(clk);
> -}
> -
> static void lpc18xx_regulator_disable(void *vref)
> {
> regulator_disable(vref);
> @@ -152,7 +147,7 @@ static int lpc18xx_adc_probe(struct platform_device *pdev)
> if (IS_ERR(adc->base))
> return PTR_ERR(adc->base);
>
> - adc->clk = devm_clk_get(&pdev->dev, NULL);
> + adc->clk = devm_clk_get_enabled(&pdev->dev, NULL);
> if (IS_ERR(adc->clk))
> return dev_err_probe(&pdev->dev, PTR_ERR(adc->clk),
> "error getting clock\n");
> @@ -178,17 +173,6 @@ static int lpc18xx_adc_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> - ret = clk_prepare_enable(adc->clk);
> - if (ret) {
> - dev_err(&pdev->dev, "unable to enable clock\n");
> - return ret;
> - }
> -
> - ret = devm_add_action_or_reset(&pdev->dev, lpc18xx_clk_disable,
> - adc->clk);
> - if (ret)
> - return ret;
> -
> rate = clk_get_rate(adc->clk);
> clkdiv = DIV_ROUND_UP(rate, LPC18XX_ADC_CLK_TARGET);
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 06/13] iio: adc: rockchip_saradc: Benefit from devm_clk_get_enabled() to simplify
2022-08-08 20:47 ` [PATCH 06/13] iio: adc: rockchip_saradc: " Uwe Kleine-König
@ 2022-08-13 16:31 ` Jonathan Cameron
2022-08-14 19:01 ` Andy Shevchenko
0 siblings, 1 reply; 30+ messages in thread
From: Jonathan Cameron @ 2022-08-13 16:31 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Andy Shevchenko, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, kernel,
Jonathan Cameron, Nuno Sa, Michal Simek, David Wu, Simon Xue
On Mon, 8 Aug 2022 22:47:33 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> Make use of devm_clk_get_enabled() to replace some code that effectively
> open codes this new function.
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
This might have side effects as it now enables the clock before calling
the clk_set_rate(). Also changes the clock start up ordering. Neither is that
scary a change, but on really fussy hardware they might cause problems.
Add a few rock-chips people who have sent patches in last few years
to hopefully take a look or even better run a test.
Thanks,
Jonathan
> ---
> drivers/iio/adc/rockchip_saradc.c | 44 ++-----------------------------
> 1 file changed, 2 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
> index b87ea7148b58..db1190faa223 100644
> --- a/drivers/iio/adc/rockchip_saradc.c
> +++ b/drivers/iio/adc/rockchip_saradc.c
> @@ -233,20 +233,6 @@ static void rockchip_saradc_reset_controller(struct reset_control *reset)
> reset_control_deassert(reset);
> }
>
> -static void rockchip_saradc_clk_disable(void *data)
> -{
> - struct rockchip_saradc *info = data;
> -
> - clk_disable_unprepare(info->clk);
> -}
> -
> -static void rockchip_saradc_pclk_disable(void *data)
> -{
> - struct rockchip_saradc *info = data;
> -
> - clk_disable_unprepare(info->pclk);
> -}
> -
> static void rockchip_saradc_regulator_disable(void *data)
> {
> struct rockchip_saradc *info = data;
> @@ -380,12 +366,12 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
> return ret;
> }
>
> - info->pclk = devm_clk_get(&pdev->dev, "apb_pclk");
> + info->pclk = devm_clk_get_enabled(&pdev->dev, "apb_pclk");
> if (IS_ERR(info->pclk))
> return dev_err_probe(&pdev->dev, PTR_ERR(info->pclk),
> "failed to get pclk\n");
>
> - info->clk = devm_clk_get(&pdev->dev, "saradc");
> + info->clk = devm_clk_get_enabled(&pdev->dev, "saradc");
> if (IS_ERR(info->clk))
> return dev_err_probe(&pdev->dev, PTR_ERR(info->clk),
> "failed to get adc clock\n");
> @@ -427,32 +413,6 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>
> info->uv_vref = ret;
>
> - ret = clk_prepare_enable(info->pclk);
> - if (ret < 0) {
> - dev_err(&pdev->dev, "failed to enable pclk\n");
> - return ret;
> - }
> - ret = devm_add_action_or_reset(&pdev->dev,
> - rockchip_saradc_pclk_disable, info);
> - if (ret) {
> - dev_err(&pdev->dev, "failed to register devm action, %d\n",
> - ret);
> - return ret;
> - }
> -
> - ret = clk_prepare_enable(info->clk);
> - if (ret < 0) {
> - dev_err(&pdev->dev, "failed to enable converter clock\n");
> - return ret;
> - }
> - ret = devm_add_action_or_reset(&pdev->dev,
> - rockchip_saradc_clk_disable, info);
> - if (ret) {
> - dev_err(&pdev->dev, "failed to register devm action, %d\n",
> - ret);
> - return ret;
> - }
> -
> platform_set_drvdata(pdev, indio_dev);
>
> indio_dev->name = dev_name(&pdev->dev);
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 11/13] iio: frequency: admv1013: Benefit from devm_clk_get_enabled() to simplify
2022-08-08 20:47 ` [PATCH 11/13] iio: frequency: admv1013: " Uwe Kleine-König
@ 2022-08-13 16:37 ` Jonathan Cameron
2022-08-14 19:04 ` Andy Shevchenko
0 siblings, 1 reply; 30+ messages in thread
From: Jonathan Cameron @ 2022-08-13 16:37 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Andy Shevchenko, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, kernel,
Jonathan Cameron, Nuno Sa, Michal Simek
On Mon, 8 Aug 2022 22:47:38 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> Make use of devm_clk_get_enabled() to replace some code that effectively
> open codes this new function.
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Looks fine to me, but there is a subtle reordering + it does even more
non parsing stuff in a function called _parse.
Anyhow, would like Antoniu or someone else from ADI to take a quick look if
possible before I pick this one up.
> ---
> drivers/iio/frequency/admv1013.c | 15 +--------------
> 1 file changed, 1 insertion(+), 14 deletions(-)
>
> diff --git a/drivers/iio/frequency/admv1013.c b/drivers/iio/frequency/admv1013.c
> index b0e1f6571afb..1346d77df77f 100644
> --- a/drivers/iio/frequency/admv1013.c
> +++ b/drivers/iio/frequency/admv1013.c
> @@ -490,11 +490,6 @@ static int admv1013_init(struct admv1013_state *st)
> st->input_mode);
> }
>
> -static void admv1013_clk_disable(void *data)
> -{
> - clk_disable_unprepare(data);
> -}
> -
> static void admv1013_reg_disable(void *data)
> {
> regulator_disable(data);
> @@ -559,7 +554,7 @@ static int admv1013_properties_parse(struct admv1013_state *st)
> return dev_err_probe(&spi->dev, PTR_ERR(st->reg),
> "failed to get the common-mode voltage\n");
>
> - st->clkin = devm_clk_get(&spi->dev, "lo_in");
> + st->clkin = devm_clk_get_enabled(&spi->dev, "lo_in");
> if (IS_ERR(st->clkin))
> return dev_err_probe(&spi->dev, PTR_ERR(st->clkin),
> "failed to get the LO input clock\n");
> @@ -601,14 +596,6 @@ static int admv1013_probe(struct spi_device *spi)
> if (ret)
> return ret;
>
> - ret = clk_prepare_enable(st->clkin);
> - if (ret)
> - return ret;
> -
> - ret = devm_add_action_or_reset(&spi->dev, admv1013_clk_disable, st->clkin);
> - if (ret)
> - return ret;
> -
> st->nb.notifier_call = admv1013_freq_change;
> ret = devm_clk_notifier_register(&spi->dev, st->clkin, &st->nb);
> if (ret)
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 01/13] iio: adc: ad7124: Benefit from devm_clk_get_enabled() to simplify
2022-08-08 20:47 [PATCH 01/13] iio: adc: ad7124: Benefit from devm_clk_get_enabled() to simplify Uwe Kleine-König
` (11 preceding siblings ...)
2022-08-08 20:47 ` [PATCH 13/13] iio: imu: adis16475: " Uwe Kleine-König
@ 2022-08-13 16:39 ` Jonathan Cameron
12 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2022-08-13 16:39 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Andy Shevchenko, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, kernel,
Jonathan Cameron, Nuno Sa, Michal Simek
On Mon, 8 Aug 2022 22:47:28 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> Make use of devm_clk_get_enabled() to replace some code that effectively
> open codes this new function.
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Always good to have a cover letter on series with lots of patches.
If nothing else it gives somewhere for replies to the whole series.
I'm a bit nervous about the ordering changes in 6 and 11 so want
to give those a little more time to get review.
Applied all the rest to what will be the togreg branch of iio.git
after a rebase on rc1. For now pushed out as testing for 0-day
to see if it can find any problems we missed.
Thanks,
Jonathan
> ---
> drivers/iio/adc/ad7124.c | 15 +--------------
> 1 file changed, 1 insertion(+), 14 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
> index c5b785d8b241..4088786e1026 100644
> --- a/drivers/iio/adc/ad7124.c
> +++ b/drivers/iio/adc/ad7124.c
> @@ -936,11 +936,6 @@ static void ad7124_reg_disable(void *r)
> regulator_disable(r);
> }
>
> -static void ad7124_clk_disable(void *c)
> -{
> - clk_disable_unprepare(c);
> -}
> -
> static int ad7124_probe(struct spi_device *spi)
> {
> const struct ad7124_chip_info *info;
> @@ -993,18 +988,10 @@ static int ad7124_probe(struct spi_device *spi)
> return ret;
> }
>
> - st->mclk = devm_clk_get(&spi->dev, "mclk");
> + st->mclk = devm_clk_get_enabled(&spi->dev, "mclk");
> if (IS_ERR(st->mclk))
> return PTR_ERR(st->mclk);
>
> - ret = clk_prepare_enable(st->mclk);
> - if (ret < 0)
> - return ret;
> -
> - ret = devm_add_action_or_reset(&spi->dev, ad7124_clk_disable, st->mclk);
> - if (ret)
> - return ret;
> -
> ret = ad7124_soft_reset(st);
> if (ret < 0)
> return ret;
>
> base-commit: 8b3d743fc9e2542822826890b482afabf0e7522a
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 06/13] iio: adc: rockchip_saradc: Benefit from devm_clk_get_enabled() to simplify
2022-08-13 16:31 ` Jonathan Cameron
@ 2022-08-14 19:01 ` Andy Shevchenko
2022-08-14 21:30 ` Uwe Kleine-König
0 siblings, 1 reply; 30+ messages in thread
From: Andy Shevchenko @ 2022-08-14 19:01 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Uwe Kleine-König, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, Sascha Hauer,
Jonathan Cameron, Nuno Sa, Michal Simek, David Wu, Simon Xue
On Sat, Aug 13, 2022 at 7:21 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Mon, 8 Aug 2022 22:47:33 +0200
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
>
> > Make use of devm_clk_get_enabled() to replace some code that effectively
> > open codes this new function.
> >
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>
> This might have side effects as it now enables the clock before calling
> the clk_set_rate(). Also changes the clock start up ordering. Neither is that
> scary a change, but on really fussy hardware they might cause problems.
>
> Add a few rock-chips people who have sent patches in last few years
> to hopefully take a look or even better run a test.
I believe you found a bug in the patch. The possible solutions are:
- not take the patch
- disable and re-enable clock around clk_set_rate()
IIRC clk_set_rate() will spit a WARN if clock is enabled.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 11/13] iio: frequency: admv1013: Benefit from devm_clk_get_enabled() to simplify
2022-08-13 16:37 ` Jonathan Cameron
@ 2022-08-14 19:04 ` Andy Shevchenko
2022-08-20 11:45 ` Jonathan Cameron
0 siblings, 1 reply; 30+ messages in thread
From: Andy Shevchenko @ 2022-08-14 19:04 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Uwe Kleine-König, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, Sascha Hauer,
Jonathan Cameron, Nuno Sa, Michal Simek
On Sat, Aug 13, 2022 at 7:26 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Mon, 8 Aug 2022 22:47:38 +0200
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
>
> > Make use of devm_clk_get_enabled() to replace some code that effectively
> > open codes this new function.
> >
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Looks fine to me, but there is a subtle reordering + it does even more
> non parsing stuff in a function called _parse.
>
> Anyhow, would like Antoniu or someone else from ADI to take a quick look if
> possible before I pick this one up.
...
> > - st->clkin = devm_clk_get(&spi->dev, "lo_in");
> > + st->clkin = devm_clk_get_enabled(&spi->dev, "lo_in");
> > if (IS_ERR(st->clkin))
> > return dev_err_probe(&spi->dev, PTR_ERR(st->clkin),
> > "failed to get the LO input clock\n");
So it seems better to drop above and...
> > - ret = clk_prepare_enable(st->clkin);
> > - if (ret)
> > - return ret;
> > -
> > - ret = devm_add_action_or_reset(&spi->dev, admv1013_clk_disable, st->clkin);
> > - if (ret)
> > - return ret;
...put a call here. This will make parse() look more parse.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 06/13] iio: adc: rockchip_saradc: Benefit from devm_clk_get_enabled() to simplify
2022-08-14 19:01 ` Andy Shevchenko
@ 2022-08-14 21:30 ` Uwe Kleine-König
2022-08-15 7:30 ` Andy Shevchenko
0 siblings, 1 reply; 30+ messages in thread
From: Uwe Kleine-König @ 2022-08-14 21:30 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, Lars-Peter Clausen, Heiko Stuebner,
Michael Hennerich, linux-iio, Jonathan Cameron,
Anand Ashok Dumbre, Vladimir Zapolskiy, Paul Cercueil,
Antoniu Miclaus, Sascha Hauer, Simon Xue, Nuno Sa, Michal Simek,
David Wu
[-- Attachment #1: Type: text/plain, Size: 1566 bytes --]
Hello,
[dropped Tomislav Denis from Cc: as the address bounces]
On Sun, Aug 14, 2022 at 10:01:08PM +0300, Andy Shevchenko wrote:
> On Sat, Aug 13, 2022 at 7:21 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Mon, 8 Aug 2022 22:47:33 +0200
> > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> >
> > > Make use of devm_clk_get_enabled() to replace some code that effectively
> > > open codes this new function.
> > >
> > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> >
> > This might have side effects as it now enables the clock before calling
> > the clk_set_rate(). Also changes the clock start up ordering. Neither is that
> > scary a change, but on really fussy hardware they might cause problems.
> >
> > Add a few rock-chips people who have sent patches in last few years
> > to hopefully take a look or even better run a test.
>
> I believe you found a bug in the patch. The possible solutions are:
> - not take the patch
> - disable and re-enable clock around clk_set_rate()
>
> IIRC clk_set_rate() will spit a WARN if clock is enabled.
You mean in general? I think that's wrong. There might be some clks that
do that, but I'd consider them strange. If you ask me, calling
clk_set_rate() for a *disabled* clk is the strange concept ...
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 06/13] iio: adc: rockchip_saradc: Benefit from devm_clk_get_enabled() to simplify
2022-08-14 21:30 ` Uwe Kleine-König
@ 2022-08-15 7:30 ` Andy Shevchenko
2022-08-15 7:41 ` Uwe Kleine-König
0 siblings, 1 reply; 30+ messages in thread
From: Andy Shevchenko @ 2022-08-15 7:30 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jonathan Cameron, Lars-Peter Clausen, Heiko Stuebner,
Michael Hennerich, linux-iio, Jonathan Cameron,
Anand Ashok Dumbre, Vladimir Zapolskiy, Paul Cercueil,
Antoniu Miclaus, Sascha Hauer, Simon Xue, Nuno Sa, Michal Simek,
David Wu
On Mon, Aug 15, 2022 at 12:31 AM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> Hello,
>
> [dropped Tomislav Denis from Cc: as the address bounces]
>
> On Sun, Aug 14, 2022 at 10:01:08PM +0300, Andy Shevchenko wrote:
> > On Sat, Aug 13, 2022 at 7:21 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > >
> > > On Mon, 8 Aug 2022 22:47:33 +0200
> > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> > >
> > > > Make use of devm_clk_get_enabled() to replace some code that effectively
> > > > open codes this new function.
> > > >
> > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > >
> > > This might have side effects as it now enables the clock before calling
> > > the clk_set_rate(). Also changes the clock start up ordering. Neither is that
> > > scary a change, but on really fussy hardware they might cause problems.
> > >
> > > Add a few rock-chips people who have sent patches in last few years
> > > to hopefully take a look or even better run a test.
> >
> > I believe you found a bug in the patch. The possible solutions are:
> > - not take the patch
> > - disable and re-enable clock around clk_set_rate()
> >
> > IIRC clk_set_rate() will spit a WARN if clock is enabled.
>
> You mean in general? I think that's wrong. There might be some clks that
> do that, but I'd consider them strange. If you ask me, calling
> clk_set_rate() for a *disabled* clk is the strange concept ...
I think it's correct from the logic and electrical perspective. That's
why the preparation and enablement are separated in CCF. But please
double check, because I can't remember everything by heart.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 06/13] iio: adc: rockchip_saradc: Benefit from devm_clk_get_enabled() to simplify
2022-08-15 7:30 ` Andy Shevchenko
@ 2022-08-15 7:41 ` Uwe Kleine-König
2022-08-16 8:20 ` Andy Shevchenko
0 siblings, 1 reply; 30+ messages in thread
From: Uwe Kleine-König @ 2022-08-15 7:41 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Lars-Peter Clausen, Heiko Stuebner, Michael Hennerich, linux-iio,
David Wu, Anand Ashok Dumbre, Vladimir Zapolskiy, Paul Cercueil,
Antoniu Miclaus, Sascha Hauer, Jonathan Cameron, Nuno Sa,
Michal Simek, Jonathan Cameron, Simon Xue
[-- Attachment #1: Type: text/plain, Size: 2354 bytes --]
On Mon, Aug 15, 2022 at 10:30:45AM +0300, Andy Shevchenko wrote:
> On Mon, Aug 15, 2022 at 12:31 AM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> >
> > Hello,
> >
> > [dropped Tomislav Denis from Cc: as the address bounces]
> >
> > On Sun, Aug 14, 2022 at 10:01:08PM +0300, Andy Shevchenko wrote:
> > > On Sat, Aug 13, 2022 at 7:21 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > > >
> > > > On Mon, 8 Aug 2022 22:47:33 +0200
> > > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> > > >
> > > > > Make use of devm_clk_get_enabled() to replace some code that effectively
> > > > > open codes this new function.
> > > > >
> > > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > >
> > > > This might have side effects as it now enables the clock before calling
> > > > the clk_set_rate(). Also changes the clock start up ordering. Neither is that
> > > > scary a change, but on really fussy hardware they might cause problems.
> > > >
> > > > Add a few rock-chips people who have sent patches in last few years
> > > > to hopefully take a look or even better run a test.
> > >
> > > I believe you found a bug in the patch. The possible solutions are:
> > > - not take the patch
> > > - disable and re-enable clock around clk_set_rate()
> > >
> > > IIRC clk_set_rate() will spit a WARN if clock is enabled.
> >
> > You mean in general? I think that's wrong. There might be some clks that
> > do that, but I'd consider them strange. If you ask me, calling
> > clk_set_rate() for a *disabled* clk is the strange concept ...
>
> I think it's correct from the logic and electrical perspective. That's
> why the preparation and enablement are separated in CCF. But please
> double check, because I can't remember everything by heart.
In my book the separation is done because "enabling" has to sleep for
some clks (e.g. PLLs) while a sleeping clk_enable() is bad for various
use cases and most clks don't sleep for enabling.
Anyhow, I don't have the hardware, so I'd rely on someone who can
actually test that.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 06/13] iio: adc: rockchip_saradc: Benefit from devm_clk_get_enabled() to simplify
2022-08-15 7:41 ` Uwe Kleine-König
@ 2022-08-16 8:20 ` Andy Shevchenko
2022-08-16 8:27 ` Lars-Peter Clausen
0 siblings, 1 reply; 30+ messages in thread
From: Andy Shevchenko @ 2022-08-16 8:20 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Lars-Peter Clausen, Heiko Stuebner, Michael Hennerich, linux-iio,
David Wu, Anand Ashok Dumbre, Vladimir Zapolskiy, Paul Cercueil,
Antoniu Miclaus, Sascha Hauer, Jonathan Cameron, Nuno Sa,
Michal Simek, Jonathan Cameron, Simon Xue
On Mon, Aug 15, 2022 at 10:42 AM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Mon, Aug 15, 2022 at 10:30:45AM +0300, Andy Shevchenko wrote:
> > On Mon, Aug 15, 2022 at 12:31 AM Uwe Kleine-König
> > <u.kleine-koenig@pengutronix.de> wrote:
> > > On Sun, Aug 14, 2022 at 10:01:08PM +0300, Andy Shevchenko wrote:
> > > > On Sat, Aug 13, 2022 at 7:21 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > > > > On Mon, 8 Aug 2022 22:47:33 +0200
> > > > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> > > > >
> > > > > > Make use of devm_clk_get_enabled() to replace some code that effectively
> > > > > > open codes this new function.
> > > > > >
> > > > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > > >
> > > > > This might have side effects as it now enables the clock before calling
> > > > > the clk_set_rate(). Also changes the clock start up ordering. Neither is that
> > > > > scary a change, but on really fussy hardware they might cause problems.
> > > > >
> > > > > Add a few rock-chips people who have sent patches in last few years
> > > > > to hopefully take a look or even better run a test.
> > > >
> > > > I believe you found a bug in the patch. The possible solutions are:
> > > > - not take the patch
> > > > - disable and re-enable clock around clk_set_rate()
> > > >
> > > > IIRC clk_set_rate() will spit a WARN if clock is enabled.
> > >
> > > You mean in general? I think that's wrong. There might be some clks that
> > > do that, but I'd consider them strange. If you ask me, calling
> > > clk_set_rate() for a *disabled* clk is the strange concept ...
> >
> > I think it's correct from the logic and electrical perspective. That's
> > why the preparation and enablement are separated in CCF. But please
> > double check, because I can't remember everything by heart.
>
> In my book the separation is done because "enabling" has to sleep for
> some clks (e.g. PLLs) while a sleeping clk_enable() is bad for various
> use cases and most clks don't sleep for enabling.
Yeah, but the idea of changing clock rate on the fly may produce
interesting side-effects on hardware level (with PLL latencies to lock
the phase and possible glitches). So, changing clock against enabled
hardware (not in reset / power off state) seems not a good idea.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 06/13] iio: adc: rockchip_saradc: Benefit from devm_clk_get_enabled() to simplify
2022-08-16 8:20 ` Andy Shevchenko
@ 2022-08-16 8:27 ` Lars-Peter Clausen
2022-08-16 8:38 ` Lars-Peter Clausen
0 siblings, 1 reply; 30+ messages in thread
From: Lars-Peter Clausen @ 2022-08-16 8:27 UTC (permalink / raw)
To: Andy Shevchenko, Uwe Kleine-König
Cc: Heiko Stuebner, Michael Hennerich, linux-iio, David Wu,
Anand Ashok Dumbre, Vladimir Zapolskiy, Paul Cercueil,
Antoniu Miclaus, Sascha Hauer, Jonathan Cameron, Nuno Sa,
Michal Simek, Jonathan Cameron, Simon Xue
On 8/16/22 10:20, Andy Shevchenko wrote:
> On Mon, Aug 15, 2022 at 10:42 AM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
>> On Mon, Aug 15, 2022 at 10:30:45AM +0300, Andy Shevchenko wrote:
>>> On Mon, Aug 15, 2022 at 12:31 AM Uwe Kleine-König
>>> <u.kleine-koenig@pengutronix.de> wrote:
>>>> On Sun, Aug 14, 2022 at 10:01:08PM +0300, Andy Shevchenko wrote:
>>>>> On Sat, Aug 13, 2022 at 7:21 PM Jonathan Cameron <jic23@kernel.org> wrote:
>>>>>> On Mon, 8 Aug 2022 22:47:33 +0200
>>>>>> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
>>>>>>
>>>>>>> Make use of devm_clk_get_enabled() to replace some code that effectively
>>>>>>> open codes this new function.
>>>>>>>
>>>>>>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>>>>>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>>>>>> This might have side effects as it now enables the clock before calling
>>>>>> the clk_set_rate(). Also changes the clock start up ordering. Neither is that
>>>>>> scary a change, but on really fussy hardware they might cause problems.
>>>>>>
>>>>>> Add a few rock-chips people who have sent patches in last few years
>>>>>> to hopefully take a look or even better run a test.
>>>>> I believe you found a bug in the patch. The possible solutions are:
>>>>> - not take the patch
>>>>> - disable and re-enable clock around clk_set_rate()
>>>>>
>>>>> IIRC clk_set_rate() will spit a WARN if clock is enabled.
>>>> You mean in general? I think that's wrong. There might be some clks that
>>>> do that, but I'd consider them strange. If you ask me, calling
>>>> clk_set_rate() for a *disabled* clk is the strange concept ...
>>> I think it's correct from the logic and electrical perspective. That's
>>> why the preparation and enablement are separated in CCF. But please
>>> double check, because I can't remember everything by heart.
>> In my book the separation is done because "enabling" has to sleep for
>> some clks (e.g. PLLs) while a sleeping clk_enable() is bad for various
>> use cases and most clks don't sleep for enabling.
> Yeah, but the idea of changing clock rate on the fly may produce
> interesting side-effects on hardware level (with PLL latencies to lock
> the phase and possible glitches). So, changing clock against enabled
> hardware (not in reset / power off state) seems not a good idea.
The clk_set_rate() API will internally disable the clock, if the clock
chip requires it. See `CLK_SET_RATE_GATE` flag.
But I tend to agree, the better idiom is to `set_rate` we should do that
before `enable`. This will avoid any unintentional glitches on the clock
signal
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 06/13] iio: adc: rockchip_saradc: Benefit from devm_clk_get_enabled() to simplify
2022-08-16 8:27 ` Lars-Peter Clausen
@ 2022-08-16 8:38 ` Lars-Peter Clausen
2022-08-20 11:42 ` Jonathan Cameron
0 siblings, 1 reply; 30+ messages in thread
From: Lars-Peter Clausen @ 2022-08-16 8:38 UTC (permalink / raw)
To: Andy Shevchenko, Uwe Kleine-König
Cc: Heiko Stuebner, Michael Hennerich, linux-iio, David Wu,
Anand Ashok Dumbre, Vladimir Zapolskiy, Paul Cercueil,
Antoniu Miclaus, Sascha Hauer, Jonathan Cameron, Nuno Sa,
Michal Simek, Jonathan Cameron, Simon Xue
On 8/16/22 10:27, Lars-Peter Clausen wrote:
> On 8/16/22 10:20, Andy Shevchenko wrote:
>> On Mon, Aug 15, 2022 at 10:42 AM Uwe Kleine-König
>> <u.kleine-koenig@pengutronix.de> wrote:
>>> On Mon, Aug 15, 2022 at 10:30:45AM +0300, Andy Shevchenko wrote:
>>>> On Mon, Aug 15, 2022 at 12:31 AM Uwe Kleine-König
>>>> <u.kleine-koenig@pengutronix.de> wrote:
>>>>> On Sun, Aug 14, 2022 at 10:01:08PM +0300, Andy Shevchenko wrote:
>>>>>> On Sat, Aug 13, 2022 at 7:21 PM Jonathan Cameron
>>>>>> <jic23@kernel.org> wrote:
>>>>>>> On Mon, 8 Aug 2022 22:47:33 +0200
>>>>>>> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
>>>>>>>
>>>>>>>> Make use of devm_clk_get_enabled() to replace some code that
>>>>>>>> effectively
>>>>>>>> open codes this new function.
>>>>>>>>
>>>>>>>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>>>>>>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>>>>>>> This might have side effects as it now enables the clock before
>>>>>>> calling
>>>>>>> the clk_set_rate(). Also changes the clock start up ordering.
>>>>>>> Neither is that
>>>>>>> scary a change, but on really fussy hardware they might cause
>>>>>>> problems.
>>>>>>>
>>>>>>> Add a few rock-chips people who have sent patches in last few years
>>>>>>> to hopefully take a look or even better run a test.
>>>>>> I believe you found a bug in the patch. The possible solutions are:
>>>>>> - not take the patch
>>>>>> - disable and re-enable clock around clk_set_rate()
>>>>>>
>>>>>> IIRC clk_set_rate() will spit a WARN if clock is enabled.
>>>>> You mean in general? I think that's wrong. There might be some
>>>>> clks that
>>>>> do that, but I'd consider them strange. If you ask me, calling
>>>>> clk_set_rate() for a *disabled* clk is the strange concept ...
>>>> I think it's correct from the logic and electrical perspective. That's
>>>> why the preparation and enablement are separated in CCF. But please
>>>> double check, because I can't remember everything by heart.
>>> In my book the separation is done because "enabling" has to sleep for
>>> some clks (e.g. PLLs) while a sleeping clk_enable() is bad for various
>>> use cases and most clks don't sleep for enabling.
>> Yeah, but the idea of changing clock rate on the fly may produce
>> interesting side-effects on hardware level (with PLL latencies to lock
>> the phase and possible glitches). So, changing clock against enabled
>> hardware (not in reset / power off state) seems not a good idea.
>
> The clk_set_rate() API will internally disable the clock, if the clock
> chip requires it. See `CLK_SET_RATE_GATE` flag.
Sorry, I misremembered. If `CLK_SET_RATE_GATE` is set `set_rate` will
return an error if the clock is enabled when `set_rate` is called.
>
> But I tend to agree, the better idiom is to `set_rate` we should do
> that before `enable`. This will avoid any unintentional glitches on
> the clock signal
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 06/13] iio: adc: rockchip_saradc: Benefit from devm_clk_get_enabled() to simplify
2022-08-16 8:38 ` Lars-Peter Clausen
@ 2022-08-20 11:42 ` Jonathan Cameron
0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2022-08-20 11:42 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Andy Shevchenko, Uwe Kleine-König, Heiko Stuebner,
Michael Hennerich, linux-iio, David Wu, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, Sascha Hauer,
Jonathan Cameron, Nuno Sa, Michal Simek, Simon Xue
On Tue, 16 Aug 2022 10:38:41 +0200
Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 8/16/22 10:27, Lars-Peter Clausen wrote:
> > On 8/16/22 10:20, Andy Shevchenko wrote:
> >> On Mon, Aug 15, 2022 at 10:42 AM Uwe Kleine-König
> >> <u.kleine-koenig@pengutronix.de> wrote:
> >>> On Mon, Aug 15, 2022 at 10:30:45AM +0300, Andy Shevchenko wrote:
> >>>> On Mon, Aug 15, 2022 at 12:31 AM Uwe Kleine-König
> >>>> <u.kleine-koenig@pengutronix.de> wrote:
> >>>>> On Sun, Aug 14, 2022 at 10:01:08PM +0300, Andy Shevchenko wrote:
> >>>>>> On Sat, Aug 13, 2022 at 7:21 PM Jonathan Cameron
> >>>>>> <jic23@kernel.org> wrote:
> >>>>>>> On Mon, 8 Aug 2022 22:47:33 +0200
> >>>>>>> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> >>>>>>>
> >>>>>>>> Make use of devm_clk_get_enabled() to replace some code that
> >>>>>>>> effectively
> >>>>>>>> open codes this new function.
> >>>>>>>>
> >>>>>>>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> >>>>>>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> >>>>>>> This might have side effects as it now enables the clock before
> >>>>>>> calling
> >>>>>>> the clk_set_rate(). Also changes the clock start up ordering.
> >>>>>>> Neither is that
> >>>>>>> scary a change, but on really fussy hardware they might cause
> >>>>>>> problems.
> >>>>>>>
> >>>>>>> Add a few rock-chips people who have sent patches in last few years
> >>>>>>> to hopefully take a look or even better run a test.
> >>>>>> I believe you found a bug in the patch. The possible solutions are:
> >>>>>> - not take the patch
> >>>>>> - disable and re-enable clock around clk_set_rate()
> >>>>>>
> >>>>>> IIRC clk_set_rate() will spit a WARN if clock is enabled.
> >>>>> You mean in general? I think that's wrong. There might be some
> >>>>> clks that
> >>>>> do that, but I'd consider them strange. If you ask me, calling
> >>>>> clk_set_rate() for a *disabled* clk is the strange concept ...
> >>>> I think it's correct from the logic and electrical perspective. That's
> >>>> why the preparation and enablement are separated in CCF. But please
> >>>> double check, because I can't remember everything by heart.
> >>> In my book the separation is done because "enabling" has to sleep for
> >>> some clks (e.g. PLLs) while a sleeping clk_enable() is bad for various
> >>> use cases and most clks don't sleep for enabling.
> >> Yeah, but the idea of changing clock rate on the fly may produce
> >> interesting side-effects on hardware level (with PLL latencies to lock
> >> the phase and possible glitches). So, changing clock against enabled
> >> hardware (not in reset / power off state) seems not a good idea.
> >
> > The clk_set_rate() API will internally disable the clock, if the clock
> > chip requires it. See `CLK_SET_RATE_GATE` flag.
> Sorry, I misremembered. If `CLK_SET_RATE_GATE` is set `set_rate` will
> return an error if the clock is enabled when `set_rate` is called.
Thanks for chasing that down.
In that case I definitely don't want to take this without input from those
who can test it!
Jonathan
> >
> > But I tend to agree, the better idiom is to `set_rate` we should do
> > that before `enable`. This will avoid any unintentional glitches on
> > the clock signal
> >
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 11/13] iio: frequency: admv1013: Benefit from devm_clk_get_enabled() to simplify
2022-08-14 19:04 ` Andy Shevchenko
@ 2022-08-20 11:45 ` Jonathan Cameron
0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2022-08-20 11:45 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Uwe Kleine-König, Tomislav Denis, Lars-Peter Clausen,
Heiko Stuebner, Michael Hennerich, linux-iio, Anand Ashok Dumbre,
Vladimir Zapolskiy, Paul Cercueil, Antoniu Miclaus, Sascha Hauer,
Jonathan Cameron, Nuno Sa, Michal Simek
On Sun, 14 Aug 2022 22:04:35 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Sat, Aug 13, 2022 at 7:26 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Mon, 8 Aug 2022 22:47:38 +0200
> > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> >
> > > Make use of devm_clk_get_enabled() to replace some code that effectively
> > > open codes this new function.
> > >
> > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > Looks fine to me, but there is a subtle reordering + it does even more
> > non parsing stuff in a function called _parse.
> >
> > Anyhow, would like Antoniu or someone else from ADI to take a quick look if
> > possible before I pick this one up.
>
> ...
>
> > > - st->clkin = devm_clk_get(&spi->dev, "lo_in");
> > > + st->clkin = devm_clk_get_enabled(&spi->dev, "lo_in");
> > > if (IS_ERR(st->clkin))
> > > return dev_err_probe(&spi->dev, PTR_ERR(st->clkin),
> > > "failed to get the LO input clock\n");
>
> So it seems better to drop above and...
>
> > > - ret = clk_prepare_enable(st->clkin);
> > > - if (ret)
> > > - return ret;
> > > -
> > > - ret = devm_add_action_or_reset(&spi->dev, admv1013_clk_disable, st->clkin);
> > > - if (ret)
> > > - return ret;
>
> ...put a call here. This will make parse() look more parse.
>
Agreed. I was thinking we actually did something (like get the rate) inbetween but
there isn't anything like that.
Whilst it's sort of true that a devm_clk_get_enabled() is part of the firmware
parsing, that's also true of the regulators that are already outside that function.
Hence the rearrangement Andy suggests makes sense to me.
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2022-08-20 11:34 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-08 20:47 [PATCH 01/13] iio: adc: ad7124: Benefit from devm_clk_get_enabled() to simplify Uwe Kleine-König
2022-08-08 20:47 ` [PATCH 02/13] iio: adc: ad7768-1: " Uwe Kleine-König
2022-08-08 20:47 ` [PATCH 03/13] iio: adc: ad9467: " Uwe Kleine-König
2022-08-08 20:47 ` [PATCH 04/13] iio: adc: ad9467: Benefit from devm_clk_get_prepared() " Uwe Kleine-König
2022-08-08 21:01 ` Andy Shevchenko
2022-08-09 5:37 ` [PATCH 04/13] iio: adc: ingenic-adc: " Uwe Kleine-König
2022-08-13 16:19 ` Jonathan Cameron
2022-08-08 20:47 ` [PATCH 05/13] iio: adc: lpc18xx: Benefit from devm_clk_get_enabled() " Uwe Kleine-König
2022-08-13 16:25 ` Jonathan Cameron
2022-08-08 20:47 ` [PATCH 06/13] iio: adc: rockchip_saradc: " Uwe Kleine-König
2022-08-13 16:31 ` Jonathan Cameron
2022-08-14 19:01 ` Andy Shevchenko
2022-08-14 21:30 ` Uwe Kleine-König
2022-08-15 7:30 ` Andy Shevchenko
2022-08-15 7:41 ` Uwe Kleine-König
2022-08-16 8:20 ` Andy Shevchenko
2022-08-16 8:27 ` Lars-Peter Clausen
2022-08-16 8:38 ` Lars-Peter Clausen
2022-08-20 11:42 ` Jonathan Cameron
2022-08-08 20:47 ` [PATCH 07/13] iio: adc: ti-ads131e08: " Uwe Kleine-König
2022-08-08 20:47 ` [PATCH 08/13] iio: adc: xilinx-ams: " Uwe Kleine-König
2022-08-08 20:47 ` [PATCH 09/13] iio: adc: xilinx-xadc: " Uwe Kleine-König
2022-08-08 20:47 ` [PATCH 10/13] iio: frequency: adf4371: " Uwe Kleine-König
2022-08-08 20:47 ` [PATCH 11/13] iio: frequency: admv1013: " Uwe Kleine-König
2022-08-13 16:37 ` Jonathan Cameron
2022-08-14 19:04 ` Andy Shevchenko
2022-08-20 11:45 ` Jonathan Cameron
2022-08-08 20:47 ` [PATCH 12/13] iio: frequency: adrf6780: " Uwe Kleine-König
2022-08-08 20:47 ` [PATCH 13/13] iio: imu: adis16475: " Uwe Kleine-König
2022-08-13 16:39 ` [PATCH 01/13] iio: adc: ad7124: " Jonathan Cameron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.