Re: [PATCH 1/1] drivers:rtc: fix return value check in mpfs_rtc_probe()

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2112 bytes --]

On Mon, Jul 17, 2023 at 10:47:05PM +0800, Yuanjun Gong wrote:
> in mpfs_rtc_probe, devm_clk_get

In the future, please add ()s to the end of function names.

> may fail, and its return value
> should be checked before passing to clk_get_rate().
>

Fixes: 0b31d703598d ("rtc: Add driver for Microchip PolarFire SoC")
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

I don't think this is the only think not quite right about the handling
of the "rtcref" clock, since nothing ever calls enable on it, instead
relying on its dual role as the timebase...

One thing to note, I assume you are using a script to generate v1
patches, since the PCI patch you sent me the other day also had this
problem, but "drivers: rtc:" does not match the prefix used by other
patches for this driver:
 rtc: mpfs: Convert to platform remove callback returning void
 rtc: mpfs: Use devm_clk_get_enabled() helper
 rtc: mpfs: Remove printing of stray CR
 rtc: mpfs: remove 'pending' variable from mpfs_rtc_wakeup_irq_handler()
 rtc: Add driver for Microchip PolarFire SoC

Thanks,
Conor.

> Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@163.com>
> ---
>  drivers/rtc/rtc-mpfs.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/rtc/rtc-mpfs.c b/drivers/rtc/rtc-mpfs.c
> index 5b96a6d39210..c126df40c343 100644
> --- a/drivers/rtc/rtc-mpfs.c
> +++ b/drivers/rtc/rtc-mpfs.c
> @@ -257,7 +257,11 @@ static int mpfs_rtc_probe(struct platform_device *pdev)
>  	}
>  
>  	/* prescaler hardware adds 1 to reg value */
> -	prescaler = clk_get_rate(devm_clk_get(&pdev->dev, "rtcref")) - 1;
> +	clk = devm_clk_get(&pdev->dev, "rtcref");
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +	prescaler = clk_get_rate(clk) - 1;
> +
>  	if (prescaler > MAX_PRESCALER_COUNT) {
>  		dev_dbg(&pdev->dev, "invalid prescaler %lu\n", prescaler);
>  		return -EINVAL;
> -- 
> 2.17.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv