| Message ID | 20210324201223.75921-1-u.kleine-koenig@pengutronix.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | i2c: imx: Simplify using devm_clk_get_prepared() | expand |
On Wed, Mar 24, 2021 at 09:12: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/i2c/busses/i2c-imx.c | 11 ++--------- > 1 file changed, 2 insertions(+), 9 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c > index b80fdc1f0092..c0e18a6caa38 100644 > --- a/drivers/i2c/busses/i2c-imx.c > +++ b/drivers/i2c/busses/i2c-imx.c > @@ -1405,16 +1405,10 @@ static int i2c_imx_probe(struct platform_device *pdev) > ACPI_COMPANION_SET(&i2c_imx->adapter.dev, ACPI_COMPANION(&pdev->dev)); > > /* Get I2C clock */ > - i2c_imx->clk = devm_clk_get(&pdev->dev, NULL); > + i2c_imx->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
On Wed, Mar 24, 2021 at 09:22:32PM +0100, Uwe Kleine-König wrote: > On Wed, Mar 24, 2021 at 09:12: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/i2c/busses/i2c-imx.c | 11 ++--------- > > 1 file changed, 2 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c > > index b80fdc1f0092..c0e18a6caa38 100644 > > --- a/drivers/i2c/busses/i2c-imx.c > > +++ b/drivers/i2c/busses/i2c-imx.c > > @@ -1405,16 +1405,10 @@ static int i2c_imx_probe(struct platform_device *pdev) > > ACPI_COMPANION_SET(&i2c_imx->adapter.dev, ACPI_COMPANION(&pdev->dev)); > > > > /* Get I2C clock */ > > - i2c_imx->clk = devm_clk_get(&pdev->dev, NULL); > > + i2c_imx->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). Hi, please send fixed version :) regards, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi "Uwe,
I love your patch! Perhaps something to improve:
[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on clk/clk-next shawnguo/for-next v5.12-rc4 next-20210326]
[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/i2c-imx-Simplify-using-devm_clk_get_prepared/20210325-041454
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: arm64-randconfig-r033-20210326 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project f490a5969bd52c8a48586f134ff8f02ccbb295b3)
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
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/0day-ci/linux/commit/2241b5e30667c72568ec9dc31ab14475bb04a408
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Uwe-Kleine-K-nig/i2c-imx-Simplify-using-devm_clk_get_prepared/20210325-041454
git checkout 2241b5e30667c72568ec9dc31ab14475bb04a408
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/i2c/busses/i2c-imx.c:1408:17: error: implicit declaration of function 'devm_clk_get_prepared' [-Werror,-Wimplicit-function-declaration]
i2c_imx->clk = devm_clk_get_prepared(&pdev->dev, NULL);
^
>> drivers/i2c/busses/i2c-imx.c:1408:15: warning: incompatible integer to pointer conversion assigning to 'struct clk *' from 'int' [-Wint-conversion]
i2c_imx->clk = devm_clk_get_prepared(&pdev->dev, NULL);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning and 1 error generated.
vim +1408 drivers/i2c/busses/i2c-imx.c
1363
1364 static int i2c_imx_probe(struct platform_device *pdev)
1365 {
1366 struct imx_i2c_struct *i2c_imx;
1367 struct resource *res;
1368 struct imxi2c_platform_data *pdata = dev_get_platdata(&pdev->dev);
1369 void __iomem *base;
1370 int irq, ret;
1371 dma_addr_t phy_addr;
1372 const struct imx_i2c_hwdata *match;
1373
1374 dev_dbg(&pdev->dev, "<%s>\n", __func__);
1375
1376 irq = platform_get_irq(pdev, 0);
1377 if (irq < 0)
1378 return irq;
1379
1380 res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
1381 base = devm_ioremap_resource(&pdev->dev, res);
1382 if (IS_ERR(base))
1383 return PTR_ERR(base);
1384
1385 phy_addr = (dma_addr_t)res->start;
1386 i2c_imx = devm_kzalloc(&pdev->dev, sizeof(*i2c_imx), GFP_KERNEL);
1387 if (!i2c_imx)
1388 return -ENOMEM;
1389
1390 match = device_get_match_data(&pdev->dev);
1391 if (match)
1392 i2c_imx->hwdata = match;
1393 else
1394 i2c_imx->hwdata = (struct imx_i2c_hwdata *)
1395 platform_get_device_id(pdev)->driver_data;
1396
1397 /* Setup i2c_imx driver structure */
1398 strlcpy(i2c_imx->adapter.name, pdev->name, sizeof(i2c_imx->adapter.name));
1399 i2c_imx->adapter.owner = THIS_MODULE;
1400 i2c_imx->adapter.algo = &i2c_imx_algo;
1401 i2c_imx->adapter.dev.parent = &pdev->dev;
1402 i2c_imx->adapter.nr = pdev->id;
1403 i2c_imx->adapter.dev.of_node = pdev->dev.of_node;
1404 i2c_imx->base = base;
1405 ACPI_COMPANION_SET(&i2c_imx->adapter.dev, ACPI_COMPANION(&pdev->dev));
1406
1407 /* Get I2C clock */
> 1408 i2c_imx->clk = devm_clk_get_prepared(&pdev->dev, NULL);
1409 if (IS_ERR(i2c_imx->clk))
1410 return dev_err_probe(&pdev->dev, PTR_ERR(i2c_imx->clk),
1411 "can't get prepared I2C clock\n");
1412
1413 /* Init queue */
1414 init_waitqueue_head(&i2c_imx->queue);
1415
1416 /* Set up adapter data */
1417 i2c_set_adapdata(&i2c_imx->adapter, i2c_imx);
1418
1419 /* Set up platform driver data */
1420 platform_set_drvdata(pdev, i2c_imx);
1421
1422 pm_runtime_set_autosuspend_delay(&pdev->dev, I2C_PM_TIMEOUT);
1423 pm_runtime_use_autosuspend(&pdev->dev);
1424 pm_runtime_set_active(&pdev->dev);
1425 pm_runtime_enable(&pdev->dev);
1426
1427 ret = pm_runtime_get_sync(&pdev->dev);
1428 if (ret < 0)
1429 goto rpm_disable;
1430
1431 /* Request IRQ */
1432 ret = request_threaded_irq(irq, i2c_imx_isr, NULL, IRQF_SHARED,
1433 pdev->name, i2c_imx);
1434 if (ret) {
1435 dev_err(&pdev->dev, "can't claim irq %d\n", irq);
1436 goto rpm_disable;
1437 }
1438
1439 /* Set up clock divider */
1440 i2c_imx->bitrate = I2C_MAX_STANDARD_MODE_FREQ;
1441 ret = of_property_read_u32(pdev->dev.of_node,
1442 "clock-frequency", &i2c_imx->bitrate);
1443 if (ret < 0 && pdata && pdata->bitrate)
1444 i2c_imx->bitrate = pdata->bitrate;
1445 i2c_imx->clk_change_nb.notifier_call = i2c_imx_clk_notifier_call;
1446 clk_notifier_register(i2c_imx->clk, &i2c_imx->clk_change_nb);
1447 i2c_imx_set_clk(i2c_imx, clk_get_rate(i2c_imx->clk));
1448
1449 i2c_imx_reset_regs(i2c_imx);
1450
1451 /* Init optional bus recovery function */
1452 ret = i2c_imx_init_recovery_info(i2c_imx, pdev);
1453 /* Give it another chance if pinctrl used is not ready yet */
1454 if (ret == -EPROBE_DEFER)
1455 goto clk_notifier_unregister;
1456
1457 /* Add I2C adapter */
1458 ret = i2c_add_numbered_adapter(&i2c_imx->adapter);
1459 if (ret < 0)
1460 goto clk_notifier_unregister;
1461
1462 pm_runtime_mark_last_busy(&pdev->dev);
1463 pm_runtime_put_autosuspend(&pdev->dev);
1464
1465 dev_dbg(&i2c_imx->adapter.dev, "claimed irq %d\n", irq);
1466 dev_dbg(&i2c_imx->adapter.dev, "device resources: %pR\n", res);
1467 dev_dbg(&i2c_imx->adapter.dev, "adapter name: \"%s\"\n",
1468 i2c_imx->adapter.name);
1469 dev_info(&i2c_imx->adapter.dev, "IMX I2C adapter registered\n");
1470
1471 /* Init DMA config if supported */
1472 i2c_imx_dma_request(i2c_imx, phy_addr);
1473
1474 return 0; /* Return OK */
1475
1476 clk_notifier_unregister:
1477 clk_notifier_unregister(i2c_imx->clk, &i2c_imx->clk_change_nb);
1478 free_irq(irq, i2c_imx);
1479 rpm_disable:
1480 pm_runtime_put_noidle(&pdev->dev);
1481 pm_runtime_disable(&pdev->dev);
1482 pm_runtime_set_suspended(&pdev->dev);
1483 pm_runtime_dont_use_autosuspend(&pdev->dev);
1484 clk_disable_unprepare(i2c_imx->clk);
1485 return ret;
1486 }
1487
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi "Uwe,
I love your patch! Yet something to improve:
[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on clk/clk-next shawnguo/for-next v5.12-rc5 next-20210330]
[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/i2c-imx-Simplify-using-devm_clk_get_prepared/20210325-041454
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: arm-multi_v7_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/2241b5e30667c72568ec9dc31ab14475bb04a408
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Uwe-Kleine-K-nig/i2c-imx-Simplify-using-devm_clk_get_prepared/20210325-041454
git checkout 2241b5e30667c72568ec9dc31ab14475bb04a408
# 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/i2c/busses/i2c-imx.c: In function 'i2c_imx_probe':
>> drivers/i2c/busses/i2c-imx.c:1408:17: error: implicit declaration of function 'devm_clk_get_prepared' [-Werror=implicit-function-declaration]
1408 | i2c_imx->clk = devm_clk_get_prepared(&pdev->dev, NULL);
| ^~~~~~~~~~~~~~~~~~~~~
drivers/i2c/busses/i2c-imx.c:1408:15: warning: assignment to 'struct clk *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
1408 | i2c_imx->clk = devm_clk_get_prepared(&pdev->dev, NULL);
| ^
cc1: some warnings being treated as errors
vim +/devm_clk_get_prepared +1408 drivers/i2c/busses/i2c-imx.c
1363
1364 static int i2c_imx_probe(struct platform_device *pdev)
1365 {
1366 struct imx_i2c_struct *i2c_imx;
1367 struct resource *res;
1368 struct imxi2c_platform_data *pdata = dev_get_platdata(&pdev->dev);
1369 void __iomem *base;
1370 int irq, ret;
1371 dma_addr_t phy_addr;
1372 const struct imx_i2c_hwdata *match;
1373
1374 dev_dbg(&pdev->dev, "<%s>\n", __func__);
1375
1376 irq = platform_get_irq(pdev, 0);
1377 if (irq < 0)
1378 return irq;
1379
1380 res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
1381 base = devm_ioremap_resource(&pdev->dev, res);
1382 if (IS_ERR(base))
1383 return PTR_ERR(base);
1384
1385 phy_addr = (dma_addr_t)res->start;
1386 i2c_imx = devm_kzalloc(&pdev->dev, sizeof(*i2c_imx), GFP_KERNEL);
1387 if (!i2c_imx)
1388 return -ENOMEM;
1389
1390 match = device_get_match_data(&pdev->dev);
1391 if (match)
1392 i2c_imx->hwdata = match;
1393 else
1394 i2c_imx->hwdata = (struct imx_i2c_hwdata *)
1395 platform_get_device_id(pdev)->driver_data;
1396
1397 /* Setup i2c_imx driver structure */
1398 strlcpy(i2c_imx->adapter.name, pdev->name, sizeof(i2c_imx->adapter.name));
1399 i2c_imx->adapter.owner = THIS_MODULE;
1400 i2c_imx->adapter.algo = &i2c_imx_algo;
1401 i2c_imx->adapter.dev.parent = &pdev->dev;
1402 i2c_imx->adapter.nr = pdev->id;
1403 i2c_imx->adapter.dev.of_node = pdev->dev.of_node;
1404 i2c_imx->base = base;
1405 ACPI_COMPANION_SET(&i2c_imx->adapter.dev, ACPI_COMPANION(&pdev->dev));
1406
1407 /* Get I2C clock */
> 1408 i2c_imx->clk = devm_clk_get_prepared(&pdev->dev, NULL);
1409 if (IS_ERR(i2c_imx->clk))
1410 return dev_err_probe(&pdev->dev, PTR_ERR(i2c_imx->clk),
1411 "can't get prepared I2C clock\n");
1412
1413 /* Init queue */
1414 init_waitqueue_head(&i2c_imx->queue);
1415
1416 /* Set up adapter data */
1417 i2c_set_adapdata(&i2c_imx->adapter, i2c_imx);
1418
1419 /* Set up platform driver data */
1420 platform_set_drvdata(pdev, i2c_imx);
1421
1422 pm_runtime_set_autosuspend_delay(&pdev->dev, I2C_PM_TIMEOUT);
1423 pm_runtime_use_autosuspend(&pdev->dev);
1424 pm_runtime_set_active(&pdev->dev);
1425 pm_runtime_enable(&pdev->dev);
1426
1427 ret = pm_runtime_get_sync(&pdev->dev);
1428 if (ret < 0)
1429 goto rpm_disable;
1430
1431 /* Request IRQ */
1432 ret = request_threaded_irq(irq, i2c_imx_isr, NULL, IRQF_SHARED,
1433 pdev->name, i2c_imx);
1434 if (ret) {
1435 dev_err(&pdev->dev, "can't claim irq %d\n", irq);
1436 goto rpm_disable;
1437 }
1438
1439 /* Set up clock divider */
1440 i2c_imx->bitrate = I2C_MAX_STANDARD_MODE_FREQ;
1441 ret = of_property_read_u32(pdev->dev.of_node,
1442 "clock-frequency", &i2c_imx->bitrate);
1443 if (ret < 0 && pdata && pdata->bitrate)
1444 i2c_imx->bitrate = pdata->bitrate;
1445 i2c_imx->clk_change_nb.notifier_call = i2c_imx_clk_notifier_call;
1446 clk_notifier_register(i2c_imx->clk, &i2c_imx->clk_change_nb);
1447 i2c_imx_set_clk(i2c_imx, clk_get_rate(i2c_imx->clk));
1448
1449 i2c_imx_reset_regs(i2c_imx);
1450
1451 /* Init optional bus recovery function */
1452 ret = i2c_imx_init_recovery_info(i2c_imx, pdev);
1453 /* Give it another chance if pinctrl used is not ready yet */
1454 if (ret == -EPROBE_DEFER)
1455 goto clk_notifier_unregister;
1456
1457 /* Add I2C adapter */
1458 ret = i2c_add_numbered_adapter(&i2c_imx->adapter);
1459 if (ret < 0)
1460 goto clk_notifier_unregister;
1461
1462 pm_runtime_mark_last_busy(&pdev->dev);
1463 pm_runtime_put_autosuspend(&pdev->dev);
1464
1465 dev_dbg(&i2c_imx->adapter.dev, "claimed irq %d\n", irq);
1466 dev_dbg(&i2c_imx->adapter.dev, "device resources: %pR\n", res);
1467 dev_dbg(&i2c_imx->adapter.dev, "adapter name: \"%s\"\n",
1468 i2c_imx->adapter.name);
1469 dev_info(&i2c_imx->adapter.dev, "IMX I2C adapter registered\n");
1470
1471 /* Init DMA config if supported */
1472 i2c_imx_dma_request(i2c_imx, phy_addr);
1473
1474 return 0; /* Return OK */
1475
1476 clk_notifier_unregister:
1477 clk_notifier_unregister(i2c_imx->clk, &i2c_imx->clk_change_nb);
1478 free_irq(irq, i2c_imx);
1479 rpm_disable:
1480 pm_runtime_put_noidle(&pdev->dev);
1481 pm_runtime_disable(&pdev->dev);
1482 pm_runtime_set_suspended(&pdev->dev);
1483 pm_runtime_dont_use_autosuspend(&pdev->dev);
1484 clk_disable_unprepare(i2c_imx->clk);
1485 return ret;
1486 }
1487
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index b80fdc1f0092..c0e18a6caa38 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -1405,16 +1405,10 @@ static int i2c_imx_probe(struct platform_device *pdev) ACPI_COMPANION_SET(&i2c_imx->adapter.dev, ACPI_COMPANION(&pdev->dev)); /* Get I2C clock */ - i2c_imx->clk = devm_clk_get(&pdev->dev, NULL); + i2c_imx->clk = devm_clk_get_prepared(&pdev->dev, NULL); if (IS_ERR(i2c_imx->clk)) return dev_err_probe(&pdev->dev, PTR_ERR(i2c_imx->clk), - "can't get I2C clock\n"); - - ret = clk_prepare_enable(i2c_imx->clk); - if (ret) { - dev_err(&pdev->dev, "can't enable I2C clock, ret=%d\n", ret); - return ret; - } + "can't get prepared I2C clock\n"); /* Init queue */ init_waitqueue_head(&i2c_imx->queue); @@ -1517,7 +1511,6 @@ static int i2c_imx_remove(struct platform_device *pdev) irq = platform_get_irq(pdev, 0); if (irq >= 0) free_irq(irq, i2c_imx); - clk_disable_unprepare(i2c_imx->clk); pm_runtime_put_noidle(&pdev->dev); pm_runtime_disable(&pdev->dev);
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/i2c/busses/i2c-imx.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-)