linux-kernel - Re: [PATCH v2] misc: atmel-ssc: Handle return value of clk_prepare_enable and clk_prepare

Open Source and information security mailing list archives

Message-ID: <20170603221753.lfiqthuc7dxy3dmc@piout.net>
Date:   Sun, 4 Jun 2017 00:17:53 +0200
From:   Alexandre Belloni <alexandre.belloni@...e-electrons.com>
To:     Arvind Yadav <arvind.yadav.cs@...il.com>
Cc:     gregkh@...uxfoundation.org, nicolas.ferre@...rochip.com,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2] misc: atmel-ssc: Handle return value of
 clk_prepare_enable and clk_prepare

Hi,

It is getting tiring to get patches that are nor even compile tested
resulting from whatever static analysis tool you used.

This patch has almost no value and v1 was clearly wrong.

Do you realize clk_prepare and clk_prepare_enable will never fail for
the SSC?

On 02/06/2017 at 11:09:02 +0530, Arvind Yadav wrote:
> clk_prepare_enable() and clk_prepare() can fail here and
> we must check its return value.
> 
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@...il.com>
> ---
>  drivers/misc/atmel-ssc.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/misc/atmel-ssc.c b/drivers/misc/atmel-ssc.c
> index b2a0340..df34b81 100644
> --- a/drivers/misc/atmel-ssc.c
> +++ b/drivers/misc/atmel-ssc.c
> @@ -30,6 +30,7 @@ struct ssc_device *ssc_request(unsigned int ssc_num)
>  {
>  	int ssc_valid = 0;
>  	struct ssc_device *ssc;
> +	int ret;
>  
>  	spin_lock(&user_lock);
>  	list_for_each_entry(ssc, &ssc_list, list) {
> @@ -60,7 +61,11 @@ struct ssc_device *ssc_request(unsigned int ssc_num)
>  	ssc->user++;
>  	spin_unlock(&user_lock);
>  
> -	clk_prepare(ssc->clk);
> +	ret = clk_prepare(ssc->clk);
> +	if (ret) {
> +		pr_err("Failed to prepare clock\n");
> +		return ERR_PTR(ret);
> +	}
>  
>  	return ssc;
>  }
> @@ -195,6 +200,7 @@ static int ssc_probe(struct platform_device *pdev)
>  	struct resource *regs;
>  	struct ssc_device *ssc;
>  	const struct atmel_ssc_platform_data *plat_dat;
> +	int ret;
>  
>  	ssc = devm_kzalloc(&pdev->dev, sizeof(struct ssc_device), GFP_KERNEL);
>  	if (!ssc) {
> @@ -229,7 +235,9 @@ static int ssc_probe(struct platform_device *pdev)
>  	}
>  
>  	/* disable all interrupts */
> -	clk_prepare_enable(ssc->clk);
> +	ret = clk_prepare_enable(ssc->clk);
> +	if (ret)
> +		return ret;
>  	ssc_writel(ssc->regs, IDR, -1);
>  	ssc_readl(ssc->regs, SR);
>  	clk_disable_unprepare(ssc->clk);
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives