| Message ID | 20210324201723.76299-1-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | spi: davinci: Simplify using devm_clk_get_prepared() | expand |
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;
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
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 --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);
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(-)