| Message ID | 20230822131237.1022815-1-lizetao1@huawei.com |
|---|---|
| Headers | show |
| Series | spi: Use devm_clk_get_*() helper function to simplify the drivers. | expand |
On Tue, Aug 22, 2023 at 09:12:23PM +0800, Li Zetao wrote: > Since commit 7ef9651e9792 ("clk: Provide new devm_clk helpers for prepared > and enabled clocks"), devm_clk_get() and clk_prepare_enable() can now be > replaced by devm_clk_get_enabled() when driver enables (and possibly > prepares) the clocks for the whole lifetime of the device. Moreover, it is > no longer necessary to unprepare and disable the clocks explicitly. > > Signed-off-by: Li Zetao <lizetao1@huawei.com> LGTM. Thanks! Acked-by: Serge Semin <fancer.lancer@gmail.com> -Serge(y) > --- > drivers/spi/spi-dw-bt1.c | 23 +++++------------------ > 1 file changed, 5 insertions(+), 18 deletions(-) > > diff --git a/drivers/spi/spi-dw-bt1.c b/drivers/spi/spi-dw-bt1.c > index 5e1c01822967..5391bcac305c 100644 > --- a/drivers/spi/spi-dw-bt1.c > +++ b/drivers/spi/spi-dw-bt1.c > @@ -269,43 +269,32 @@ static int dw_spi_bt1_probe(struct platform_device *pdev) > > dws->paddr = mem->start; > > - dwsbt1->clk = devm_clk_get(&pdev->dev, NULL); > + dwsbt1->clk = devm_clk_get_enabled(&pdev->dev, NULL); > if (IS_ERR(dwsbt1->clk)) > return PTR_ERR(dwsbt1->clk); > > - ret = clk_prepare_enable(dwsbt1->clk); > - if (ret) > - return ret; > - > dws->bus_num = pdev->id; > dws->reg_io_width = 4; > dws->max_freq = clk_get_rate(dwsbt1->clk); > - if (!dws->max_freq) { > - ret = -EINVAL; > - goto err_disable_clk; > - } > + if (!dws->max_freq) > + return -EINVAL; > > init_func = device_get_match_data(&pdev->dev); > ret = init_func(pdev, dwsbt1); > if (ret) > - goto err_disable_clk; > + return ret; > > pm_runtime_enable(&pdev->dev); > > ret = dw_spi_add_host(&pdev->dev, dws); > if (ret) { > pm_runtime_disable(&pdev->dev); > - goto err_disable_clk; > + return ret; > } > > platform_set_drvdata(pdev, dwsbt1); > > return 0; > - > -err_disable_clk: > - clk_disable_unprepare(dwsbt1->clk); > - > - return ret; > } > > static void dw_spi_bt1_remove(struct platform_device *pdev) > @@ -315,8 +304,6 @@ static void dw_spi_bt1_remove(struct platform_device *pdev) > dw_spi_remove_host(&dwsbt1->dws); > > pm_runtime_disable(&pdev->dev); > - > - clk_disable_unprepare(dwsbt1->clk); > } > > static const struct of_device_id dw_spi_bt1_of_match[] = { > -- > 2.34.1 >
On Tue, 22 Aug 2023 21:12:12 +0800 Li Zetao <lizetao1@huawei.com> wrote: > Commit 7ef9651e9792 ("clk: Provide new devm_clk helpers for prepared > and enabled clocks") provides a new helper function for prepared and > enabled clocks when a driver keeps a clock prepared (or enabled) during > the whole lifetime of the driver. So where drivers get clocks and enable > them immediately, it can be combined into a single function > devm_clk_get_*(). Moreover, the unprepare and disable function > has been registered to devm_clk_state, and before devm_clk_state is > released, the clocks will be unprepareed and disable, so it is unnecessary > to unprepare and disable clocks explicitly when remove drivers or in the > error handling path. For all except 2, 12 and 24 they look good to me and I don't think there are any other ordering issues of the sort we tend to see in devm conversions where things get turned off later than in pre devm version. So for those.. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > Li Zetao (25): > spi: ar934x: Use helper function devm_clk_get_enabled() > spi: armada-3700: Use helper function devm_clk_get_prepared() > spi: aspeed: Use helper function devm_clk_get_enabled() > spi: ath79: Use helper function devm_clk_get_enabled() > spi: spi-axi-spi-engine: Use helper function devm_clk_get_enabled() > spi: bcm2835: Use helper function devm_clk_get_enabled() > spi: bcm2835aux: Use helper function devm_clk_get_enabled() > spi: spi-cadence: Use helper function devm_clk_get_enabled() > spi: spi-cavium-thunderx: Use helper function devm_clk_get_enabled() > spi: davinci: Use helper function devm_clk_get_enabled() > spi: dw-bt1: Use helper function devm_clk_get_enabled() > spi: dw-mmio: Use helper function devm_clk_get_*() > spi: spi-fsl-dspi: Use helper function devm_clk_get_enabled() > spi: lantiq-ssc: Use helper function devm_clk_get_enabled() > spi: meson-spicc: Use helper function devm_clk_get_enabled() > spi: spi-meson-spifc: Use helper function devm_clk_get_enabled() > spi: microchip-core-qspi: Use helper function devm_clk_get_enabled() > spi: microchip-core: Use helper function devm_clk_get_enabled() > spi: mtk-snfi: Use helper function devm_clk_get_enabled() > spi: npcm-fiu: Use helper function devm_clk_get_enabled() > spi: orion: Use helper function devm_clk_get_enabled() > spi: pic32-sqi: Use helper function devm_clk_get_enabled() > spi: pic32: Use helper function devm_clk_get_enabled() > spi: spl022: Use helper function devm_clk_get_enabled() > spi: rockchip: Use helper function devm_clk_get_enabled() > > drivers/spi/spi-ar934x.c | 22 ++-------- > drivers/spi/spi-armada-3700.c | 18 ++------ > drivers/spi/spi-aspeed-smc.c | 16 +------ > drivers/spi/spi-ath79.c | 11 +---- > drivers/spi/spi-axi-spi-engine.c | 25 +++-------- > drivers/spi/spi-bcm2835.c | 11 +---- > drivers/spi/spi-bcm2835aux.c | 23 ++-------- > drivers/spi/spi-cadence.c | 23 ++-------- > drivers/spi/spi-cavium-thunderx.c | 8 +--- > drivers/spi/spi-davinci.c | 11 +---- > drivers/spi/spi-dw-bt1.c | 23 +++------- > drivers/spi/spi-dw-mmio.c | 20 +++------ > drivers/spi/spi-fsl-dspi.c | 12 ++---- > drivers/spi/spi-lantiq-ssc.c | 10 +---- > drivers/spi/spi-meson-spicc.c | 33 +++------------ > drivers/spi/spi-meson-spifc.c | 17 ++------ > drivers/spi/spi-microchip-core-qspi.c | 29 +++---------- > drivers/spi/spi-microchip-core.c | 9 +--- > drivers/spi/spi-mtk-snfi.c | 61 ++++----------------------- > drivers/spi/spi-npcm-fiu.c | 14 ++---- > drivers/spi/spi-orion.c | 11 +---- > drivers/spi/spi-pic32-sqi.c | 27 ++---------- > drivers/spi/spi-pic32.c | 8 +--- > drivers/spi/spi-pl022.c | 21 +++------ > drivers/spi/spi-rockchip.c | 30 +++---------- > 25 files changed, 88 insertions(+), 405 deletions(-) >
On Wed, Aug 23, 2023 at 09:39:25PM +0800, Li Zetao wrote: > Since commit 7ef9651e9792 ("clk: Provide new devm_clk helpers for prepared > and enabled clocks"), devm_clk_get() and clk_prepare_enable() can now be > replaced by devm_clk_get_enabled() when driver enables (and possibly > prepares) the clocks for the whole lifetime of the device. Moreover, it is > no longer necessary to unprepare and disable the clocks explicitly. Also, > devm_clk_get_optional() and clk_prepare_enable() can now be replaced by > devm_clk_get_optional_enabled(). Moreover, the lable "out_clk" no longer > makes sense, rename it to "out_reset". > > Signed-off-by: Li Zetao <lizetao1@huawei.com> > --- > v1 -> v2: Return directly instead of calling reset_control_deassert() > before the reset control handler has been requested. And use the > "out_reset" label instead of "out" before calling pm_runtime_enable(). LGTM. Thanks! Acked-by: Serge Semin <fancer.lancer@gmail.com> -Serge(y) > > drivers/spi/spi-dw-mmio.c | 31 +++++++++---------------------- > 1 file changed, 9 insertions(+), 22 deletions(-) > > diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c > index 805264c9c65c..46801189a651 100644 > --- a/drivers/spi/spi-dw-mmio.c > +++ b/drivers/spi/spi-dw-mmio.c > @@ -340,29 +340,20 @@ static int dw_spi_mmio_probe(struct platform_device *pdev) > if (dws->irq < 0) > return dws->irq; /* -ENXIO */ > > - dwsmmio->clk = devm_clk_get(&pdev->dev, NULL); > + dwsmmio->clk = devm_clk_get_enabled(&pdev->dev, NULL); > if (IS_ERR(dwsmmio->clk)) > return PTR_ERR(dwsmmio->clk); > - ret = clk_prepare_enable(dwsmmio->clk); > - if (ret) > - return ret; > > /* Optional clock needed to access the registers */ > - dwsmmio->pclk = devm_clk_get_optional(&pdev->dev, "pclk"); > - if (IS_ERR(dwsmmio->pclk)) { > - ret = PTR_ERR(dwsmmio->pclk); > - goto out_clk; > - } > - ret = clk_prepare_enable(dwsmmio->pclk); > - if (ret) > - goto out_clk; > + dwsmmio->pclk = devm_clk_get_optional_enabled(&pdev->dev, "pclk"); > + if (IS_ERR(dwsmmio->pclk)) > + return PTR_ERR(dwsmmio->pclk); > > /* find an optional reset controller */ > dwsmmio->rstc = devm_reset_control_get_optional_exclusive(&pdev->dev, "spi"); > - if (IS_ERR(dwsmmio->rstc)) { > - ret = PTR_ERR(dwsmmio->rstc); > - goto out_clk; > - } > + if (IS_ERR(dwsmmio->rstc)) > + return PTR_ERR(dwsmmio->rstc); > + > reset_control_deassert(dwsmmio->rstc); > > dws->bus_num = pdev->id; > @@ -383,7 +374,7 @@ static int dw_spi_mmio_probe(struct platform_device *pdev) > if (init_func) { > ret = init_func(pdev, dwsmmio); > if (ret) > - goto out; > + goto out_reset; > } > > pm_runtime_enable(&pdev->dev); > @@ -397,9 +388,7 @@ static int dw_spi_mmio_probe(struct platform_device *pdev) > > out: > pm_runtime_disable(&pdev->dev); > - clk_disable_unprepare(dwsmmio->pclk); > -out_clk: > - clk_disable_unprepare(dwsmmio->clk); > +out_reset: > reset_control_assert(dwsmmio->rstc); > > return ret; > @@ -411,8 +400,6 @@ static void dw_spi_mmio_remove(struct platform_device *pdev) > > dw_spi_remove_host(&dwsmmio->dws); > pm_runtime_disable(&pdev->dev); > - clk_disable_unprepare(dwsmmio->pclk); > - clk_disable_unprepare(dwsmmio->clk); > reset_control_assert(dwsmmio->rstc); > } > > -- > 2.34.1 >
On Wed, Aug 23, 2023 at 09:39:13PM +0800, Li Zetao wrote: > Commit 7ef9651e9792 ("clk: Provide new devm_clk helpers for prepared > and enabled clocks") provides a new helper function for prepared and > enabled clocks when a driver keeps a clock prepared (or enabled) during > the whole lifetime of the driver. So where drivers get clocks and enable > them immediately, it can be combined into a single function Please don't send new patches in reply to old patches or serieses, this makes it harder for both people and tools to understand what is going on - it can bury things in mailboxes and make it difficult to keep track of what current patches are, both for the new patches and the old ones.
On Wed, 23 Aug 2023 21:39:15 +0800 Li Zetao <lizetao1@huawei.com> wrote: > Since commit 7ef9651e9792 ("clk: Provide new devm_clk helpers for prepared > and enabled clocks"), devm_clk_get() and clk_prepare() can now be replaced > by devm_clk_get_prepared() when driver prepares the clocks for the whole > lifetime of the device. Moreover, it is no longer necessary to unprepare > the clocks explicitly. > > Signed-off-by: Li Zetao <lizetao1@huawei.com> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > v1 -> v2: Drop the empty remove function a3700_spi_remove(). > > drivers/spi/spi-armada-3700.c | 23 +++-------------------- > 1 file changed, 3 insertions(+), 20 deletions(-) > > diff --git a/drivers/spi/spi-armada-3700.c b/drivers/spi/spi-armada-3700.c > index 0103ac0158c0..3c9ed412932f 100644 > --- a/drivers/spi/spi-armada-3700.c > +++ b/drivers/spi/spi-armada-3700.c > @@ -865,18 +865,12 @@ static int a3700_spi_probe(struct platform_device *pdev) > > init_completion(&spi->done); > > - spi->clk = devm_clk_get(dev, NULL); > + spi->clk = devm_clk_get_prepared(dev, NULL); > if (IS_ERR(spi->clk)) { > dev_err(dev, "could not find clk: %ld\n", PTR_ERR(spi->clk)); > goto error; > } > > - ret = clk_prepare(spi->clk); > - if (ret) { > - dev_err(dev, "could not prepare clk: %d\n", ret); > - goto error; > - } > - > host->max_speed_hz = min_t(unsigned long, A3700_SPI_MAX_SPEED_HZ, > clk_get_rate(spi->clk)); > host->min_speed_hz = DIV_ROUND_UP(clk_get_rate(spi->clk), > @@ -888,40 +882,29 @@ static int a3700_spi_probe(struct platform_device *pdev) > dev_name(dev), host); > if (ret) { > dev_err(dev, "could not request IRQ: %d\n", ret); > - goto error_clk; > + goto error; > } > > ret = devm_spi_register_controller(dev, host); > if (ret) { > dev_err(dev, "Failed to register host\n"); > - goto error_clk; > + goto error; > } > > return 0; > > -error_clk: > - clk_unprepare(spi->clk); > error: > spi_controller_put(host); > out: > return ret; > } > > -static void a3700_spi_remove(struct platform_device *pdev) > -{ > - struct spi_controller *host = platform_get_drvdata(pdev); > - struct a3700_spi *spi = spi_controller_get_devdata(host); > - > - clk_unprepare(spi->clk); > -} > - > static struct platform_driver a3700_spi_driver = { > .driver = { > .name = DRIVER_NAME, > .of_match_table = of_match_ptr(a3700_spi_dt_ids), > }, > .probe = a3700_spi_probe, > - .remove_new = a3700_spi_remove, > }; > > module_platform_driver(a3700_spi_driver);
On Wed, 23 Aug 2023 17:20:12 +0300 Serge Semin <fancer.lancer@gmail.com> wrote: > On Wed, Aug 23, 2023 at 09:39:25PM +0800, Li Zetao wrote: > > Since commit 7ef9651e9792 ("clk: Provide new devm_clk helpers for prepared > > and enabled clocks"), devm_clk_get() and clk_prepare_enable() can now be > > replaced by devm_clk_get_enabled() when driver enables (and possibly > > prepares) the clocks for the whole lifetime of the device. Moreover, it is > > no longer necessary to unprepare and disable the clocks explicitly. Also, > > devm_clk_get_optional() and clk_prepare_enable() can now be replaced by > > devm_clk_get_optional_enabled(). Moreover, the lable "out_clk" no longer > > makes sense, rename it to "out_reset". > > > > Signed-off-by: Li Zetao <lizetao1@huawei.com> > > --- > > v1 -> v2: Return directly instead of calling reset_control_deassert() > > before the reset control handler has been requested. And use the > > "out_reset" label instead of "out" before calling pm_runtime_enable(). > > LGTM. Thanks! > Acked-by: Serge Semin <fancer.lancer@gmail.com> Agreed - looks much better now. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > -Serge(y) > > > > > drivers/spi/spi-dw-mmio.c | 31 +++++++++---------------------- > > 1 file changed, 9 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c > > index 805264c9c65c..46801189a651 100644 > > --- a/drivers/spi/spi-dw-mmio.c > > +++ b/drivers/spi/spi-dw-mmio.c > > @@ -340,29 +340,20 @@ static int dw_spi_mmio_probe(struct platform_device *pdev) > > if (dws->irq < 0) > > return dws->irq; /* -ENXIO */ > > > > - dwsmmio->clk = devm_clk_get(&pdev->dev, NULL); > > + dwsmmio->clk = devm_clk_get_enabled(&pdev->dev, NULL); > > if (IS_ERR(dwsmmio->clk)) > > return PTR_ERR(dwsmmio->clk); > > - ret = clk_prepare_enable(dwsmmio->clk); > > - if (ret) > > - return ret; > > > > /* Optional clock needed to access the registers */ > > - dwsmmio->pclk = devm_clk_get_optional(&pdev->dev, "pclk"); > > - if (IS_ERR(dwsmmio->pclk)) { > > - ret = PTR_ERR(dwsmmio->pclk); > > - goto out_clk; > > - } > > - ret = clk_prepare_enable(dwsmmio->pclk); > > - if (ret) > > - goto out_clk; > > + dwsmmio->pclk = devm_clk_get_optional_enabled(&pdev->dev, "pclk"); > > + if (IS_ERR(dwsmmio->pclk)) > > + return PTR_ERR(dwsmmio->pclk); > > > > /* find an optional reset controller */ > > dwsmmio->rstc = devm_reset_control_get_optional_exclusive(&pdev->dev, "spi"); > > - if (IS_ERR(dwsmmio->rstc)) { > > - ret = PTR_ERR(dwsmmio->rstc); > > - goto out_clk; > > - } > > + if (IS_ERR(dwsmmio->rstc)) > > + return PTR_ERR(dwsmmio->rstc); > > + > > reset_control_deassert(dwsmmio->rstc); > > > > dws->bus_num = pdev->id; > > @@ -383,7 +374,7 @@ static int dw_spi_mmio_probe(struct platform_device *pdev) > > if (init_func) { > > ret = init_func(pdev, dwsmmio); > > if (ret) > > - goto out; > > + goto out_reset; > > } > > > > pm_runtime_enable(&pdev->dev); > > @@ -397,9 +388,7 @@ static int dw_spi_mmio_probe(struct platform_device *pdev) > > > > out: > > pm_runtime_disable(&pdev->dev); > > - clk_disable_unprepare(dwsmmio->pclk); > > -out_clk: > > - clk_disable_unprepare(dwsmmio->clk); > > +out_reset: > > reset_control_assert(dwsmmio->rstc); > > > > return ret; > > @@ -411,8 +400,6 @@ static void dw_spi_mmio_remove(struct platform_device *pdev) > > > > dw_spi_remove_host(&dwsmmio->dws); > > pm_runtime_disable(&pdev->dev); > > - clk_disable_unprepare(dwsmmio->pclk); > > - clk_disable_unprepare(dwsmmio->clk); > > reset_control_assert(dwsmmio->rstc); > > } > > > > -- > > 2.34.1 > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
Commit 7ef9651e9792 ("clk: Provide new devm_clk helpers for prepared and enabled clocks") provides a new helper function for prepared and enabled clocks when a driver keeps a clock prepared (or enabled) during the whole lifetime of the driver. So where drivers get clocks and enable them immediately, it can be combined into a single function devm_clk_get_*(). Moreover, the unprepare and disable function has been registered to devm_clk_state, and before devm_clk_state is released, the clocks will be unprepareed and disable, so it is unnecessary to unprepare and disable clocks explicitly when remove drivers or in the error handling path. Li Zetao (25): spi: ar934x: Use helper function devm_clk_get_enabled() spi: armada-3700: Use helper function devm_clk_get_prepared() spi: aspeed: Use helper function devm_clk_get_enabled() spi: ath79: Use helper function devm_clk_get_enabled() spi: spi-axi-spi-engine: Use helper function devm_clk_get_enabled() spi: bcm2835: Use helper function devm_clk_get_enabled() spi: bcm2835aux: Use helper function devm_clk_get_enabled() spi: spi-cadence: Use helper function devm_clk_get_enabled() spi: spi-cavium-thunderx: Use helper function devm_clk_get_enabled() spi: davinci: Use helper function devm_clk_get_enabled() spi: dw-bt1: Use helper function devm_clk_get_enabled() spi: dw-mmio: Use helper function devm_clk_get_*() spi: spi-fsl-dspi: Use helper function devm_clk_get_enabled() spi: lantiq-ssc: Use helper function devm_clk_get_enabled() spi: meson-spicc: Use helper function devm_clk_get_enabled() spi: spi-meson-spifc: Use helper function devm_clk_get_enabled() spi: microchip-core-qspi: Use helper function devm_clk_get_enabled() spi: microchip-core: Use helper function devm_clk_get_enabled() spi: mtk-snfi: Use helper function devm_clk_get_enabled() spi: npcm-fiu: Use helper function devm_clk_get_enabled() spi: orion: Use helper function devm_clk_get_enabled() spi: pic32-sqi: Use helper function devm_clk_get_enabled() spi: pic32: Use helper function devm_clk_get_enabled() spi: spl022: Use helper function devm_clk_get_enabled() spi: rockchip: Use helper function devm_clk_get_enabled() drivers/spi/spi-ar934x.c | 22 ++-------- drivers/spi/spi-armada-3700.c | 18 ++------ drivers/spi/spi-aspeed-smc.c | 16 +------ drivers/spi/spi-ath79.c | 11 +---- drivers/spi/spi-axi-spi-engine.c | 25 +++-------- drivers/spi/spi-bcm2835.c | 11 +---- drivers/spi/spi-bcm2835aux.c | 23 ++-------- drivers/spi/spi-cadence.c | 23 ++-------- drivers/spi/spi-cavium-thunderx.c | 8 +--- drivers/spi/spi-davinci.c | 11 +---- drivers/spi/spi-dw-bt1.c | 23 +++------- drivers/spi/spi-dw-mmio.c | 20 +++------ drivers/spi/spi-fsl-dspi.c | 12 ++---- drivers/spi/spi-lantiq-ssc.c | 10 +---- drivers/spi/spi-meson-spicc.c | 33 +++------------ drivers/spi/spi-meson-spifc.c | 17 ++------ drivers/spi/spi-microchip-core-qspi.c | 29 +++---------- drivers/spi/spi-microchip-core.c | 9 +--- drivers/spi/spi-mtk-snfi.c | 61 ++++----------------------- drivers/spi/spi-npcm-fiu.c | 14 ++---- drivers/spi/spi-orion.c | 11 +---- drivers/spi/spi-pic32-sqi.c | 27 ++---------- drivers/spi/spi-pic32.c | 8 +--- drivers/spi/spi-pl022.c | 21 +++------ drivers/spi/spi-rockchip.c | 30 +++---------- 25 files changed, 88 insertions(+), 405 deletions(-)