diff mbox series

spi: davinci: Simplify using devm_clk_get_prepared()

Message ID 20210324201723.76299-1-u.kleine-koenig@pengutronix.de
State Superseded
Headers show
Series spi: davinci: Simplify using devm_clk_get_prepared() | expand

Commit Message

Uwe Kleine-König March 24, 2021, 8:17 p.m. UTC
devm_clk_get_prepared returns the clk already prepared and the
automatically called cleanup cares for unpreparing. So simplify .probe
and .remove accordingly.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

this simplification depends on a patch set that introduces
devm_clk_get_prepared() and friends.

The most recent version of this patch set can be found at

	https://lore.kernel.org/r/20210301135053.1462168-1-u.kleine-koenig@pengutronix.de

Unfortunately I didn't get any feedback at all from the clk maintainers
on it, so I try to make other maintainers aware of it in the expectation
that the simplifications are welcome and so lure the clk maintainers to
share their thoughts.

Best regards
Uwe

 drivers/spi/spi-davinci.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

Comments

Uwe Kleine-König March 24, 2021, 8:22 p.m. UTC | #1
On Wed, Mar 24, 2021 at 09:17:23PM +0100, Uwe Kleine-König wrote:
> devm_clk_get_prepared returns the clk already prepared and the
> automatically called cleanup cares for unpreparing. So simplify .probe
> and .remove accordingly.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> this simplification depends on a patch set that introduces
> devm_clk_get_prepared() and friends.
> 
> The most recent version of this patch set can be found at
> 
> 	https://lore.kernel.org/r/20210301135053.1462168-1-u.kleine-koenig@pengutronix.de
> 
> Unfortunately I didn't get any feedback at all from the clk maintainers
> on it, so I try to make other maintainers aware of it in the expectation
> that the simplifications are welcome and so lure the clk maintainers to
> share their thoughts.
> 
> Best regards
> Uwe
> 
>  drivers/spi/spi-davinci.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
> index 7453a1dbbc06..c170bccf9710 100644
> --- a/drivers/spi/spi-davinci.c
> +++ b/drivers/spi/spi-davinci.c
> @@ -936,14 +936,11 @@ static int davinci_spi_probe(struct platform_device *pdev)
>  
>  	dspi->bitbang.master = master;
>  
> -	dspi->clk = devm_clk_get(&pdev->dev, NULL);
> +	dspi->clk = devm_clk_get_prepared(&pdev->dev, NULL);

oops, I got that wrong, this must be devm_clk_get_enabled, not
devm_clk_get_prepared. So if the clk patches go in, please let me resend
a fixed patch (or adapt yourself, whatever you prefer).

Best regards
Uwe

>  	if (IS_ERR(dspi->clk)) {
>  		ret = -ENODEV;
>  		goto free_master;
>  	}
> -	ret = clk_prepare_enable(dspi->clk);
> -	if (ret)
> -		goto free_master;
kernel test robot March 30, 2021, 4:09 a.m. UTC | #2
Hi "Uwe,

I love your patch! Yet something to improve:

[auto build test ERROR on spi/for-next]
[also build test ERROR on v5.12-rc5 next-20210329]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Uwe-Kleine-K-nig/spi-davinci-Simplify-using-devm_clk_get_prepared/20210325-041955
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: arm-defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/d4207ccf698f3daec6f45ba37439f303cd20196c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Uwe-Kleine-K-nig/spi-davinci-Simplify-using-devm_clk_get_prepared/20210325-041955
        git checkout d4207ccf698f3daec6f45ba37439f303cd20196c
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/spi/spi-davinci.c: In function 'davinci_spi_probe':
>> drivers/spi/spi-davinci.c:939:14: error: implicit declaration of function 'devm_clk_get_prepared' [-Werror=implicit-function-declaration]

     939 |  dspi->clk = devm_clk_get_prepared(&pdev->dev, NULL);
         |              ^~~~~~~~~~~~~~~~~~~~~
   drivers/spi/spi-davinci.c:939:12: warning: assignment to 'struct clk *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     939 |  dspi->clk = devm_clk_get_prepared(&pdev->dev, NULL);
         |            ^
   cc1: some warnings being treated as errors


vim +/devm_clk_get_prepared +939 drivers/spi/spi-davinci.c

   856	
   857	/**
   858	 * davinci_spi_probe - probe function for SPI Master Controller
   859	 * @pdev: platform_device structure which contains plateform specific data
   860	 *
   861	 * According to Linux Device Model this function will be invoked by Linux
   862	 * with platform_device struct which contains the device specific info.
   863	 * This function will map the SPI controller's memory, register IRQ,
   864	 * Reset SPI controller and setting its registers to default value.
   865	 * It will invoke spi_bitbang_start to create work queue so that client driver
   866	 * can register transfer method to work queue.
   867	 */
   868	static int davinci_spi_probe(struct platform_device *pdev)
   869	{
   870		struct spi_master *master;
   871		struct davinci_spi *dspi;
   872		struct davinci_spi_platform_data *pdata;
   873		struct resource *r;
   874		int ret = 0;
   875		u32 spipc0;
   876	
   877		master = spi_alloc_master(&pdev->dev, sizeof(struct davinci_spi));
   878		if (master == NULL) {
   879			ret = -ENOMEM;
   880			goto err;
   881		}
   882	
   883		platform_set_drvdata(pdev, master);
   884	
   885		dspi = spi_master_get_devdata(master);
   886	
   887		if (dev_get_platdata(&pdev->dev)) {
   888			pdata = dev_get_platdata(&pdev->dev);
   889			dspi->pdata = *pdata;
   890		} else {
   891			/* update dspi pdata with that from the DT */
   892			ret = spi_davinci_get_pdata(pdev, dspi);
   893			if (ret < 0)
   894				goto free_master;
   895		}
   896	
   897		/* pdata in dspi is now updated and point pdata to that */
   898		pdata = &dspi->pdata;
   899	
   900		dspi->bytes_per_word = devm_kcalloc(&pdev->dev,
   901						    pdata->num_chipselect,
   902						    sizeof(*dspi->bytes_per_word),
   903						    GFP_KERNEL);
   904		if (dspi->bytes_per_word == NULL) {
   905			ret = -ENOMEM;
   906			goto free_master;
   907		}
   908	
   909		r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
   910		if (r == NULL) {
   911			ret = -ENOENT;
   912			goto free_master;
   913		}
   914	
   915		dspi->pbase = r->start;
   916	
   917		dspi->base = devm_ioremap_resource(&pdev->dev, r);
   918		if (IS_ERR(dspi->base)) {
   919			ret = PTR_ERR(dspi->base);
   920			goto free_master;
   921		}
   922	
   923		init_completion(&dspi->done);
   924	
   925		ret = platform_get_irq(pdev, 0);
   926		if (ret == 0)
   927			ret = -EINVAL;
   928		if (ret < 0)
   929			goto free_master;
   930		dspi->irq = ret;
   931	
   932		ret = devm_request_threaded_irq(&pdev->dev, dspi->irq, davinci_spi_irq,
   933					dummy_thread_fn, 0, dev_name(&pdev->dev), dspi);
   934		if (ret)
   935			goto free_master;
   936	
   937		dspi->bitbang.master = master;
   938	
 > 939		dspi->clk = devm_clk_get_prepared(&pdev->dev, NULL);

   940		if (IS_ERR(dspi->clk)) {
   941			ret = -ENODEV;
   942			goto free_master;
   943		}
   944	
   945		master->use_gpio_descriptors = true;
   946		master->dev.of_node = pdev->dev.of_node;
   947		master->bus_num = pdev->id;
   948		master->num_chipselect = pdata->num_chipselect;
   949		master->bits_per_word_mask = SPI_BPW_RANGE_MASK(2, 16);
   950		master->flags = SPI_MASTER_MUST_RX;
   951		master->setup = davinci_spi_setup;
   952		master->cleanup = davinci_spi_cleanup;
   953		master->can_dma = davinci_spi_can_dma;
   954	
   955		dspi->bitbang.chipselect = davinci_spi_chipselect;
   956		dspi->bitbang.setup_transfer = davinci_spi_setup_transfer;
   957		dspi->prescaler_limit = pdata->prescaler_limit;
   958		dspi->version = pdata->version;
   959	
   960		dspi->bitbang.flags = SPI_NO_CS | SPI_LSB_FIRST | SPI_LOOP | SPI_CS_WORD;
   961		if (dspi->version == SPI_VERSION_2)
   962			dspi->bitbang.flags |= SPI_READY;
   963	
   964		dspi->bitbang.txrx_bufs = davinci_spi_bufs;
   965	
   966		ret = davinci_spi_request_dma(dspi);
   967		if (ret == -EPROBE_DEFER) {
   968			goto free_master;
   969		} else if (ret) {
   970			dev_info(&pdev->dev, "DMA is not supported (%d)\n", ret);
   971			dspi->dma_rx = NULL;
   972			dspi->dma_tx = NULL;
   973		}
   974	
   975		dspi->get_rx = davinci_spi_rx_buf_u8;
   976		dspi->get_tx = davinci_spi_tx_buf_u8;
   977	
   978		/* Reset In/OUT SPI module */
   979		iowrite32(0, dspi->base + SPIGCR0);
   980		udelay(100);
   981		iowrite32(1, dspi->base + SPIGCR0);
   982	
   983		/* Set up SPIPC0.  CS and ENA init is done in davinci_spi_setup */
   984		spipc0 = SPIPC0_DIFUN_MASK | SPIPC0_DOFUN_MASK | SPIPC0_CLKFUN_MASK;
   985		iowrite32(spipc0, dspi->base + SPIPC0);
   986	
   987		if (pdata->intr_line)
   988			iowrite32(SPI_INTLVL_1, dspi->base + SPILVL);
   989		else
   990			iowrite32(SPI_INTLVL_0, dspi->base + SPILVL);
   991	
   992		iowrite32(CS_DEFAULT, dspi->base + SPIDEF);
   993	
   994		/* master mode default */
   995		set_io_bits(dspi->base + SPIGCR1, SPIGCR1_CLKMOD_MASK);
   996		set_io_bits(dspi->base + SPIGCR1, SPIGCR1_MASTER_MASK);
   997		set_io_bits(dspi->base + SPIGCR1, SPIGCR1_POWERDOWN_MASK);
   998	
   999		ret = spi_bitbang_start(&dspi->bitbang);
  1000		if (ret)
  1001			goto free_dma;
  1002	
  1003		dev_info(&pdev->dev, "Controller at 0x%p\n", dspi->base);
  1004	
  1005		return ret;
  1006	
  1007	free_dma:
  1008		if (dspi->dma_rx) {
  1009			dma_release_channel(dspi->dma_rx);
  1010			dma_release_channel(dspi->dma_tx);
  1011		}
  1012	free_master:
  1013		spi_master_put(master);
  1014	err:
  1015		return ret;
  1016	}
  1017	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Mark Brown March 30, 2021, 5:04 p.m. UTC | #3
On Wed, Mar 24, 2021 at 09:22:58PM +0100, Uwe Kleine-König wrote:

> oops, I got that wrong, this must be devm_clk_get_enabled, not

> devm_clk_get_prepared. So if the clk patches go in, please let me resend

> a fixed patch (or adapt yourself, whatever you prefer).


I'll look out for your resubmission.
diff mbox series

Patch

diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index 7453a1dbbc06..c170bccf9710 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -936,14 +936,11 @@  static int davinci_spi_probe(struct platform_device *pdev)
 
 	dspi->bitbang.master = master;
 
-	dspi->clk = devm_clk_get(&pdev->dev, NULL);
+	dspi->clk = devm_clk_get_prepared(&pdev->dev, NULL);
 	if (IS_ERR(dspi->clk)) {
 		ret = -ENODEV;
 		goto free_master;
 	}
-	ret = clk_prepare_enable(dspi->clk);
-	if (ret)
-		goto free_master;
 
 	master->use_gpio_descriptors = true;
 	master->dev.of_node = pdev->dev.of_node;
@@ -968,7 +965,7 @@  static int davinci_spi_probe(struct platform_device *pdev)
 
 	ret = davinci_spi_request_dma(dspi);
 	if (ret == -EPROBE_DEFER) {
-		goto free_clk;
+		goto free_master;
 	} else if (ret) {
 		dev_info(&pdev->dev, "DMA is not supported (%d)\n", ret);
 		dspi->dma_rx = NULL;
@@ -1012,8 +1009,6 @@  static int davinci_spi_probe(struct platform_device *pdev)
 		dma_release_channel(dspi->dma_rx);
 		dma_release_channel(dspi->dma_tx);
 	}
-free_clk:
-	clk_disable_unprepare(dspi->clk);
 free_master:
 	spi_master_put(master);
 err:
@@ -1039,8 +1034,6 @@  static int davinci_spi_remove(struct platform_device *pdev)
 
 	spi_bitbang_stop(&dspi->bitbang);
 
-	clk_disable_unprepare(dspi->clk);
-
 	if (dspi->dma_rx) {
 		dma_release_channel(dspi->dma_rx);
 		dma_release_channel(dspi->dma_tx);