diff mbox series

rtc: at91sma9: Simplify using devm_clk_get_enabled()

Message ID 20210324202711.76734-1-u.kleine-koenig@pengutronix.de
State Not Applicable
Headers show
Series rtc: at91sma9: Simplify using devm_clk_get_enabled() | expand

Commit Message

Uwe Kleine-König March 24, 2021, 8:27 p.m. UTC 
devm_clk_get_enabled() returns the clk already (prepared and) enabled
and the automatically called cleanup cares for disabling (and
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/rtc/rtc-at91sam9.c | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)

Comments

kernel test robot March 25, 2021, 1:11 a.m. UTC | #1 
Hi "Uwe,

I love your patch! Perhaps something to improve:

[auto build test WARNING on abelloni/rtc-next]
[also build test WARNING on v5.12-rc4 next-20210324]
[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/rtc-at91sma9-Simplify-using-devm_clk_get_enabled/20210325-042956
base:   https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-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/782e62ed210e25e760c5607b2ac2dbf16f56ea0f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Uwe-Kleine-K-nig/rtc-at91sma9-Simplify-using-devm_clk_get_enabled/20210325-042956
        git checkout 782e62ed210e25e760c5607b2ac2dbf16f56ea0f
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc 

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/rtc/rtc-at91sam9.c: In function 'at91_rtc_probe':
   drivers/rtc/rtc-at91sam9.c:377:14: error: implicit declaration of function 'devm_clk_get_enabled'; did you mean 'memcg_kmem_enabled'? [-Werror=implicit-function-declaration]
     377 |  rtc->sclk = devm_clk_get_enabled(&pdev->dev, NULL);
         |              ^~~~~~~~~~~~~~~~~~~~
         |              memcg_kmem_enabled
>> drivers/rtc/rtc-at91sam9.c:377:12: warning: assignment to 'struct clk *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     377 |  rtc->sclk = devm_clk_get_enabled(&pdev->dev, NULL);
         |            ^
   cc1: some warnings being treated as errors


vim +377 drivers/rtc/rtc-at91sam9.c

   331	
   332	/*
   333	 * Initialize and install RTC driver
   334	 */
   335	static int at91_rtc_probe(struct platform_device *pdev)
   336	{
   337		struct sam9_rtc	*rtc;
   338		int		ret, irq;
   339		u32		mr;
   340		unsigned int	sclk_rate;
   341		struct of_phandle_args args;
   342	
   343		irq = platform_get_irq(pdev, 0);
   344		if (irq < 0)
   345			return irq;
   346	
   347		rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
   348		if (!rtc)
   349			return -ENOMEM;
   350	
   351		spin_lock_init(&rtc->lock);
   352		rtc->irq = irq;
   353	
   354		/* platform setup code should have handled this; sigh */
   355		if (!device_can_wakeup(&pdev->dev))
   356			device_init_wakeup(&pdev->dev, 1);
   357	
   358		platform_set_drvdata(pdev, rtc);
   359	
   360		rtc->rtt = devm_platform_ioremap_resource(pdev, 0);
   361		if (IS_ERR(rtc->rtt))
   362			return PTR_ERR(rtc->rtt);
   363	
   364		ret = of_parse_phandle_with_fixed_args(pdev->dev.of_node,
   365						       "atmel,rtt-rtc-time-reg", 1, 0,
   366						       &args);
   367		if (ret)
   368			return ret;
   369	
   370		rtc->gpbr = syscon_node_to_regmap(args.np);
   371		rtc->gpbr_offset = args.args[0];
   372		if (IS_ERR(rtc->gpbr)) {
   373			dev_err(&pdev->dev, "failed to retrieve gpbr regmap, aborting.\n");
   374			return -ENOMEM;
   375		}
   376	
 > 377		rtc->sclk = devm_clk_get_enabled(&pdev->dev, NULL);
   378		if (IS_ERR(rtc->sclk))
   379			return PTR_ERR(rtc->sclk);
   380	
   381		sclk_rate = clk_get_rate(rtc->sclk);
   382		if (!sclk_rate || sclk_rate > AT91_RTT_RTPRES) {
   383			dev_err(&pdev->dev, "Invalid slow clock rate\n");
   384			return -EINVAL;
   385		}
   386	
   387		mr = rtt_readl(rtc, MR);
   388	
   389		/* unless RTT is counting at 1 Hz, re-initialize it */
   390		if ((mr & AT91_RTT_RTPRES) != sclk_rate) {
   391			mr = AT91_RTT_RTTRST | (sclk_rate & AT91_RTT_RTPRES);
   392			gpbr_writel(rtc, 0);
   393		}
   394	
   395		/* disable all interrupts (same as on shutdown path) */
   396		mr &= ~(AT91_RTT_ALMIEN | AT91_RTT_RTTINCIEN);
   397		rtt_writel(rtc, MR, mr);
   398	
   399		rtc->rtcdev = devm_rtc_allocate_device(&pdev->dev);
   400		if (IS_ERR(rtc->rtcdev)) {
   401			ret = PTR_ERR(rtc->rtcdev);
   402			return ret;
   403		}
   404	
   405		rtc->rtcdev->ops = &at91_rtc_ops;
   406		rtc->rtcdev->range_max = U32_MAX;
   407	
   408		/* register irq handler after we know what name we'll use */
   409		ret = devm_request_irq(&pdev->dev, rtc->irq, at91_rtc_interrupt,
   410				       IRQF_SHARED | IRQF_COND_SUSPEND,
   411				       dev_name(&rtc->rtcdev->dev), rtc);
   412		if (ret) {
   413			dev_dbg(&pdev->dev, "can't share IRQ %d?\n", rtc->irq);
   414			return ret;
   415		}
   416	
   417		/* NOTE:  sam9260 rev A silicon has a ROM bug which resets the
   418		 * RTT on at least some reboots.  If you have that chip, you must
   419		 * initialize the time from some external source like a GPS, wall
   420		 * clock, discrete RTC, etc
   421		 */
   422	
   423		if (gpbr_readl(rtc) == 0)
   424			dev_warn(&pdev->dev, "%s: SET TIME!\n",
   425				 dev_name(&rtc->rtcdev->dev));
   426	
   427		return devm_rtc_register_device(rtc->rtcdev);
   428	}
   429	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot March 25, 2021, 3:15 p.m. UTC | #2 
Hi "Uwe,

I love your patch! Yet something to improve:

[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on v5.12-rc4 next-20210325]
[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/rtc-at91sma9-Simplify-using-devm_clk_get_enabled/20210325-042956
base:   https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
config: arm-randconfig-r002-20210325 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 5d6b4aa80d6df62b924a12af030c5ded868ee4f1)
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 arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/782e62ed210e25e760c5607b2ac2dbf16f56ea0f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Uwe-Kleine-K-nig/rtc-at91sma9-Simplify-using-devm_clk_get_enabled/20210325-042956
        git checkout 782e62ed210e25e760c5607b2ac2dbf16f56ea0f
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm 

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

All error/warnings (new ones prefixed by >>):

>> drivers/rtc/rtc-at91sam9.c:377:14: error: implicit declaration of function 'devm_clk_get_enabled' [-Werror,-Wimplicit-function-declaration]
           rtc->sclk = devm_clk_get_enabled(&pdev->dev, NULL);
                       ^
>> drivers/rtc/rtc-at91sam9.c:377:12: warning: incompatible integer to pointer conversion assigning to 'struct clk *' from 'int' [-Wint-conversion]
           rtc->sclk = devm_clk_get_enabled(&pdev->dev, NULL);
                     ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 warning and 1 error generated.


vim +/devm_clk_get_enabled +377 drivers/rtc/rtc-at91sam9.c

   331	
   332	/*
   333	 * Initialize and install RTC driver
   334	 */
   335	static int at91_rtc_probe(struct platform_device *pdev)
   336	{
   337		struct sam9_rtc	*rtc;
   338		int		ret, irq;
   339		u32		mr;
   340		unsigned int	sclk_rate;
   341		struct of_phandle_args args;
   342	
   343		irq = platform_get_irq(pdev, 0);
   344		if (irq < 0)
   345			return irq;
   346	
   347		rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
   348		if (!rtc)
   349			return -ENOMEM;
   350	
   351		spin_lock_init(&rtc->lock);
   352		rtc->irq = irq;
   353	
   354		/* platform setup code should have handled this; sigh */
   355		if (!device_can_wakeup(&pdev->dev))
   356			device_init_wakeup(&pdev->dev, 1);
   357	
   358		platform_set_drvdata(pdev, rtc);
   359	
   360		rtc->rtt = devm_platform_ioremap_resource(pdev, 0);
   361		if (IS_ERR(rtc->rtt))
   362			return PTR_ERR(rtc->rtt);
   363	
   364		ret = of_parse_phandle_with_fixed_args(pdev->dev.of_node,
   365						       "atmel,rtt-rtc-time-reg", 1, 0,
   366						       &args);
   367		if (ret)
   368			return ret;
   369	
   370		rtc->gpbr = syscon_node_to_regmap(args.np);
   371		rtc->gpbr_offset = args.args[0];
   372		if (IS_ERR(rtc->gpbr)) {
   373			dev_err(&pdev->dev, "failed to retrieve gpbr regmap, aborting.\n");
   374			return -ENOMEM;
   375		}
   376	
 > 377		rtc->sclk = devm_clk_get_enabled(&pdev->dev, NULL);
   378		if (IS_ERR(rtc->sclk))
   379			return PTR_ERR(rtc->sclk);
   380	
   381		sclk_rate = clk_get_rate(rtc->sclk);
   382		if (!sclk_rate || sclk_rate > AT91_RTT_RTPRES) {
   383			dev_err(&pdev->dev, "Invalid slow clock rate\n");
   384			return -EINVAL;
   385		}
   386	
   387		mr = rtt_readl(rtc, MR);
   388	
   389		/* unless RTT is counting at 1 Hz, re-initialize it */
   390		if ((mr & AT91_RTT_RTPRES) != sclk_rate) {
   391			mr = AT91_RTT_RTTRST | (sclk_rate & AT91_RTT_RTPRES);
   392			gpbr_writel(rtc, 0);
   393		}
   394	
   395		/* disable all interrupts (same as on shutdown path) */
   396		mr &= ~(AT91_RTT_ALMIEN | AT91_RTT_RTTINCIEN);
   397		rtt_writel(rtc, MR, mr);
   398	
   399		rtc->rtcdev = devm_rtc_allocate_device(&pdev->dev);
   400		if (IS_ERR(rtc->rtcdev)) {
   401			ret = PTR_ERR(rtc->rtcdev);
   402			return ret;
   403		}
   404	
   405		rtc->rtcdev->ops = &at91_rtc_ops;
   406		rtc->rtcdev->range_max = U32_MAX;
   407	
   408		/* register irq handler after we know what name we'll use */
   409		ret = devm_request_irq(&pdev->dev, rtc->irq, at91_rtc_interrupt,
   410				       IRQF_SHARED | IRQF_COND_SUSPEND,
   411				       dev_name(&rtc->rtcdev->dev), rtc);
   412		if (ret) {
   413			dev_dbg(&pdev->dev, "can't share IRQ %d?\n", rtc->irq);
   414			return ret;
   415		}
   416	
   417		/* NOTE:  sam9260 rev A silicon has a ROM bug which resets the
   418		 * RTT on at least some reboots.  If you have that chip, you must
   419		 * initialize the time from some external source like a GPS, wall
   420		 * clock, discrete RTC, etc
   421		 */
   422	
   423		if (gpbr_readl(rtc) == 0)
   424			dev_warn(&pdev->dev, "%s: SET TIME!\n",
   425				 dev_name(&rtc->rtcdev->dev));
   426	
   427		return devm_rtc_register_device(rtc->rtcdev);
   428	}
   429	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/rtc/rtc-at91sam9.c b/drivers/rtc/rtc-at91sam9.c
index 2216be429ab7..b52e7bd26303 100644
--- a/drivers/rtc/rtc-at91sam9.c
+++ b/drivers/rtc/rtc-at91sam9.c
@@ -374,21 +374,14 @@  static int at91_rtc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	rtc->sclk = devm_clk_get(&pdev->dev, NULL);
+	rtc->sclk = devm_clk_get_enabled(&pdev->dev, NULL);
 	if (IS_ERR(rtc->sclk))
 		return PTR_ERR(rtc->sclk);
 
-	ret = clk_prepare_enable(rtc->sclk);
-	if (ret) {
-		dev_err(&pdev->dev, "Could not enable slow clock\n");
-		return ret;
-	}
-
 	sclk_rate = clk_get_rate(rtc->sclk);
 	if (!sclk_rate || sclk_rate > AT91_RTT_RTPRES) {
 		dev_err(&pdev->dev, "Invalid slow clock rate\n");
-		ret = -EINVAL;
-		goto err_clk;
+		return -EINVAL;
 	}
 
 	mr = rtt_readl(rtc, MR);
@@ -406,7 +399,7 @@  static int at91_rtc_probe(struct platform_device *pdev)
 	rtc->rtcdev = devm_rtc_allocate_device(&pdev->dev);
 	if (IS_ERR(rtc->rtcdev)) {
 		ret = PTR_ERR(rtc->rtcdev);
-		goto err_clk;
+		return ret;
 	}
 
 	rtc->rtcdev->ops = &at91_rtc_ops;
@@ -418,7 +411,7 @@  static int at91_rtc_probe(struct platform_device *pdev)
 			       dev_name(&rtc->rtcdev->dev), rtc);
 	if (ret) {
 		dev_dbg(&pdev->dev, "can't share IRQ %d?\n", rtc->irq);
-		goto err_clk;
+		return ret;
 	}
 
 	/* NOTE:  sam9260 rev A silicon has a ROM bug which resets the
@@ -432,11 +425,6 @@  static int at91_rtc_probe(struct platform_device *pdev)
 			 dev_name(&rtc->rtcdev->dev));
 
 	return devm_rtc_register_device(rtc->rtcdev);
-
-err_clk:
-	clk_disable_unprepare(rtc->sclk);
-
-	return ret;
 }
 
 /*
@@ -450,8 +438,6 @@  static int at91_rtc_remove(struct platform_device *pdev)
 	/* disable all interrupts */
 	rtt_writel(rtc, MR, mr & ~(AT91_RTT_ALMIEN | AT91_RTT_RTTINCIEN));
 
-	clk_disable_unprepare(rtc->sclk);
-
 	return 0;
 }